Code Quality: Break down service registration in App.xaml.cs into granular methods

[jvestell]

Description

All Dependency Injection configurations are inside a single method. The ConfigureHost() method is becoming cumbersome and less manageable as services are added. I recommend an abbreviated ConfigureHost() method:

private IHost ConfigureHost()
{
return Host.CreateDefaultBuilder()
.UseEnvironment(ApplicationService.AppEnvironment.ToString())
.ConfigureLogging( /* ... / )
.ConfigureServices(ConfigureServices) // This is where the specific registration methods get called*
.Build();
}

and improved orchestration and navigation offered by the following:

private void ConfigureServices(IServiceCollection services)
{
// General services
RegisterGeneralServices(services);

// Storage services
RegisterStorageServices(services);

// ViewModel services
RegisterViewModelServices(services);

// Additional services
RegisterAdditionalServices(services); // If you have more services to add

}

private void RegisterGeneralServices(IServiceCollection services)
{
services.AddSingleton<IUserSettingsService, UserSettingsService>();
// ... other general services
}

private void RegisterViewModelServices(IServiceCollection services)
{
services.AddSingleton();
// ... other ViewModel services
}

// other services

Concerned code

File: App.xaml.cs

	private IHost ConfigureHost()
	{
		return Host.CreateDefaultBuilder()
			.UseEnvironment(ApplicationService.AppEnvironment.ToString())
			.ConfigureLogging(builder => builder
				.AddProvider(new FileLoggerProvider(Path.Combine(ApplicationData.Current.LocalFolder.Path, "debug.log")))
				.SetMinimumLevel(LogLevel.Information))
			.ConfigureServices(services => services
				.AddSingleton<IUserSettingsService, UserSettingsService>()
				.AddSingleton<IAppearanceSettingsService, AppearanceSettingsService>(sp => new AppearanceSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IGeneralSettingsService, GeneralSettingsService>(sp => new GeneralSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IFoldersSettingsService, FoldersSettingsService>(sp => new FoldersSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IApplicationSettingsService, ApplicationSettingsService>(sp => new ApplicationSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IPreviewPaneSettingsService, PreviewPaneSettingsService>(sp => new PreviewPaneSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<ILayoutSettingsService, LayoutSettingsService>(sp => new LayoutSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IAppSettingsService, AppSettingsService>(sp => new AppSettingsService((sp.GetService<IUserSettingsService>() as UserSettingsService).GetSharingContext()))
				.AddSingleton<IFileTagsSettingsService, FileTagsSettingsService>()
				.AddSingleton<IPageContext, PageContext>()
				.AddSingleton<IContentPageContext, ContentPageContext>()
				.AddSingleton<IDisplayPageContext, DisplayPageContext>()
				.AddSingleton<IWindowContext, WindowContext>()
				.AddSingleton<IMultitaskingContext, MultitaskingContext>()
				.AddSingleton<ITagsContext, TagsContext>()
				.AddSingleton<IDialogService, DialogService>()
				.AddSingleton<IImageService, ImagingService>()
				.AddSingleton<IThreadingService, ThreadingService>()
				.AddSingleton<ILocalizationService, LocalizationService>()
				.AddSingleton<ICloudDetector, CloudDetector>()
				.AddSingleton<IFileTagsService, FileTagsService>()
				.AddSingleton<ICommandManager, CommandManager>()
				.AddSingleton<IModifiableCommandManager, ModifiableCommandManager>()
				.AddSingleton<IApplicationService, ApplicationService>()

#if UWP
.AddSingleton<IStorageService, WindowsStorageService>()
#else
.AddSingleton<IStorageService, NativeStorageService>()
#endif
.AddSingleton<IFtpStorageService, FtpStorageService>()
.AddSingleton<IAddItemService, AddItemService>()
#if STABLE || PREVIEW
.AddSingleton<IUpdateService, SideloadUpdateService>()
#else
.AddSingleton<IUpdateService, UpdateService>()
#endif
.AddSingleton<IPreviewPopupService, PreviewPopupService>()
.AddSingleton<IDateTimeFormatterFactory, DateTimeFormatterFactory>()
.AddSingleton<IDateTimeFormatter, UserDateTimeFormatter>()
.AddSingleton<IVolumeInfoFactory, VolumeInfoFactory>()
.AddSingleton<ISizeProvider, UserSizeProvider>()
.AddSingleton<IQuickAccessService, QuickAccessService>()
.AddSingleton<IResourcesService, ResourcesService>()
.AddSingleton<IJumpListService, JumpListService>()
.AddSingleton<IRemovableDrivesService, RemovableDrivesService>()
.AddSingleton<INetworkDrivesService, NetworkDrivesService>()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton()
.AddSingleton<IJsonValidator, JsonIntegrityChecker>()
).Build();
}

Gains

Breaking down service registration into smaller, focused methods improves the code's organization, readability, and maintainability.

Requirements

Refactoring IHost ConfigureHost()

Comments

No response

[0x5bfa]

To fix this situation, I was (I will again) going to introduce AppLifecycleHelper static helper class containg this too see this PR. That helper provides some methods that should be away from App class.

Besides, your opinion seem to be nice and so I would create methods, such as RegisterViewModelServices and RegisterGeneralServices into there as private methods.

[jvestell]

@0x5bfa What's the state of this issue? I can refactor service registration immediately and we can close this issue. Please let me know. @yaira2 I know that this issue was accepted and added to the task board. I'm uncertain about a dependency on @0x5bfa LifecyleApp development and how that relates to this issue.

My question, am I approved to go forth refactoring Appl.xaml.cs service registration into methods? Thanks!

[0x5bfa]

@yaira2 can you move to Ready to Build?

Once approved, you would like to create AppLifecyleHelper class in App.Helpers namespace, and put configuration class to there.

[jvestell]

@0x5bfa Okay. This means I need to write the AppLifecycleHelper class in App.Helpers. I'll put the service registration methods there. Do I understand correctly?

[0x5bfa]

Yes.

[yaira2]

@HorseBandit we're discussing this proposal and will let you know if we decide to move forward with it.

[d2dyno1]

It looks good! But I'd separate it into its own static helper class (ServiceRegistrationHelpers) to keep the code clean in App.xaml.cs

[0x5bfa]

yes id suggested him to. @d2dyno1. but for now, we can put all static classes should be moved in the same class from App class