3
\$\begingroup\$

I just finished writing the core for my WPF application and because I'm still a very beginner at software design I wanted to get some feedback about the design decisions I've made.

I've uploaded the project to Github and I'll post the most important parts here (the project is to big to be posted as a whole). To see the full code base please follow the link.

Purpose

The general purpose of the core I've written was to allow me to use dependency injection to inject directly in to the XAML System.Windows.Window throughout the application lifecycle:

public partial class MainWindow : Window
{
    public MainWindow(object myObj)
    {
        InitializeComponent();
    }
}

When specifying a StartupUri inside the WPF application for the startup window to use the WPF framework doesn't allow you to have a parameterized constructor inside the window. Because I'm a huge fan of dependency injection this was kind of a downside for me and the application core I've written has the purpose to allow me to use a startup window which gets dependencies injected in the constructor.

Component Management Module

One of the most important parts of the application is the component management module. In order to build and activate a System.Windows.Window I've written a WindowManager which itself is just a proxy for an IWindowManager which is being resolved by an IServiceResolver at runtime:

internal class WindowManager
{

    private static IWindowManager _windowManagerImpl;
    internal static IWindowManager WindowManagerImpl 
    {
        get
        {
            if (_windowManagerImpl == null)
                _windowManagerImpl = ServiceResolverBuilder.GetServiceResolver()?.Resolve<IWindowManager>();
            return _windowManagerImpl;
        }
        set
        {
            Guard.NotNull(value, nameof(value));

            _windowManagerImpl.Dispose();
            _windowManagerImpl = value;
        }
    }

    public static Window BuildWindow(Type windowType) =>
        WindowManagerImpl?.BuildWindow(windowType);

    public static TWindow BuildWindow<TWindow>() where TWindow : Window =>
        WindowManagerImpl?.BuildWindow<TWindow>();

    public static void ActivateWindow(Window window) =>
        WindowManagerImpl?.ActivateWindow(window);

    public static void ShowAsDialog(Window window) =>
        WindowManagerImpl?.ShowAsDialog(window);

}

The resolved IWindowManager depends on an IWindowBuilder implementation for creating the window and an IWindowService for activating the window.

Application Startup

On application startup I use an IStartup mainly for setting up the dependency injection container and a StartupWindowHelper which gets the startup window type from the application config. Then I call into the component service module to create and activate the window:

public partial class App : Application
{

    protected override void OnStartup(StartupEventArgs e)
    {
        RunApplicationStartup();
        BuildAndActivateStartupWindow();

        base.OnStartup(e);
    }

    private void RunApplicationStartup()
    {
        var applicationStartup = ApplicationStartupBuilder.BuildApplicationStartup();
        applicationStartup.Run();
    }

    private void BuildAndActivateStartupWindow()
    {
        var startupWindowType = StartupWindowHelper.GetStartupWindowType();

        var startupWindow = WindowManager.BuildWindow(startupWindowType);
        WindowManager.ActivateWindow(startupWindow);
    }

}

I've tried my very best writing the core as clean as possible but as I've already mentioned I'm a beginner and not very experienced with good design.

\$\endgroup\$
  • \$\begingroup\$ Seems quite a lot of code to work around newing up the parameterized window in OnStartup (and not providing a StartupUri in the xaml). Why is the myObj parameter not used though? \$\endgroup\$ – Mathieu Guindon Nov 4 '19 at 13:55
  • \$\begingroup\$ @MathieuGuindon The myObj parameter was just a sample. I've created a couple of service interfaces that will be injected into the constructor of the window instead. \$\endgroup\$ – nikstffrs Nov 4 '19 at 18:05
  • \$\begingroup\$ I can see how you would like reviewers to focus on the app startup code, but I think you'd get better and more complete feedback if you included an actual dependency being injected - doing this could easily ripple into a review of the general MVVM architecture and DI strategy employed here... whereas a hypothetical myObj dependency leaves it unclear why there's even a need to inject dependencies into the view: I for one, would expect the ViewModel to receive such dependencies. \$\endgroup\$ – Mathieu Guindon Nov 4 '19 at 18:18
  • \$\begingroup\$ For example - and maybe I'm doing this wrong, but I always override OnStartup and resolve/instantiate the dependencies there, create the ViewModel, and property-inject it manually through DataContext, e.g. var vm = new MainWindowViewModel(foo, bar); and then var window = new MainWindow { DataContext = vm };, followed by window.ShowDialog(); -- the code-behind for MainWindow simply wires up DataContextChanged, and if there's anything else in the code-behind, it's all 100% UI-specific logic, leaving the view with no dependencies other than its ViewModel. \$\endgroup\$ – Mathieu Guindon Nov 4 '19 at 18:23
2
\$\begingroup\$

Constructor injection is indeed preferable, but it's not the only way to inject dependencies - property injection is just as valid, and in the case of a WPF Window, the only dependency that should be injected, is the ViewModel - and Window already has a property exposed for that: DataContext.

Assuming a Model-View-ViewModel architecture, I cannot think of a single valid reason to constructor-inject anything into a View. The role of the View in MVVM is to be nothing more than a simple I/O device presenting data to the user, and exposing means for that user to provide inputs.

That means a View needing dependencies injected is highly suspicious, because it suggests that the View is at least partly doing the ViewModel's job, and this is going to make that logic much more difficult to test than if it were done elsewhere: the idea behind separating the View from the ViewModel isn't just to leverage the powerful XAML data bindings, it's also to get as much of the not-stricly-presentation concerns out of the View so that they can be unit-tested without popping up a UI.

Now, as I said in an earlier comment, maybe I'm doing this wrong, but I've never had issues with this approach: I remove the StartupUri in the App.xaml markup, and then edit App.xaml.cs to override OnStartup as follows:

protected override void OnStartup(StartupEventArgs e)
{
    base.OnStartup();
    // if you're using an IoC container, this is where you set it up:
    // ...

    // if you're using an IoC container, this is where you resolve your VM's dependencies:
    // ...
    // otherwise, just new it up:
    var vm = new MainWindowViewModel(/*ctor-inject dependencies manually here*/);
    var window = new MainWindow { DataContext = vm };
    window.ShowDialog();
}

The IoC configuration code should go into its own method (could be in App.xaml.cs, or in some static helper), of course. But the point remains: the view doesn't have any dependencies, other than its view model.

The MainWindow.xaml.cs code-behind might look like this:

public partial class MainWindow : Window
{
    public MainWindow()
    {
        InitializeComponent();
        DataContextChanged += MainWindow_DataContextChanged;
    }

    MainWindowViewModel ViewModel => DataContext as MainWindowViewModel;

    private void MainWindow_DataContextChanged(object sender, DependencyPropertyChangedEventArgs e)
    {
        var vm = ViewModel;
        if (vm != null)
        {
            vm.Close += HandleViewModelCloseCommand;
        }
    }

    private void HandleViewModelCloseCommand(object sender, EventArgs e)
    {
        Close();
    }
}

Everything else belongs in the ViewModel: any ICommand you might want to inject, any service, unit-of-work/repository, "business logic" worker objects - none of these are dependencies of the window itself.

Since a WPF window needs no dependencies, the base premise falls apart, and all that code can be deleted.

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.