3
\$\begingroup\$

I am fairly new to Factories and Dependency Injection in MVVM.

I've build my View(Window) factory with help from SO, and I would like to know if I had implemented everything properly(code works just fine).

I'm sorry for the length, but there was no other way for me to show my complete example.

  1. First, there is the WindowFactory class. Its purpose is to delegate the creation of windows and coupling it with their ViewModels, so that I can avoid referencing View in its respective ViewModel (following MVVM pattern).

    public class WindowFactory : IWindowFactory
    {
        private readonly IWindowManager _windowManager;
    
        public WindowFactory(IWindowManager windowManager)
        {
            this._windowManager = windowManager;
        }
    
        public void CreatePhoneWindow()
        {
            var view = new PhoneWindow();
            var viewAdapter = new ViewAdapter(view, _windowManager);
            var viewModel = new PhoneWindowViewModel(viewAdapter);
    
            _windowManager.addWindow(view);
            view.DataContext = viewModel;
            view.Show();
        }
    
        // Here are copies of the above method, one for each different window that I create
    }
    
  2. WindowManager class. Its purpose is to keep track of current open windows, and allow certain operations on them (like checking if any window is open at a time, so that I can prevent other windows from being created)

    class WindowManager : IWindowManager
    {
        private ObservableCollection<Window> _windows;
    
        public WindowManager()
        {
            _windows = new ObservableCollection<Window>();
        }
    
        public void addWindow(Window window)
        {
            if (_windows.Contains(window) == false) { _windows.Add(window); }
        }
    
        public void removeWindow(Window window)
        {
            if (_windows.Contains(window) == true) { _windows.Remove(window); }
        }
    
        public bool isAnyWindowOpen()
        {
            if (_windows.Count == 0) { return true; }
            else { return false; }
        }
    }
    
  3. ViewAdapter class. Its purpose is to handle View close commands, that are comming from inside the View's ViewModel (again, to decouple).

    class ViewAdapter : ICloseable
    {
        private readonly Window _view;
        private readonly IWindowManager _windowManager;
    
        public ViewAdapter(Window view, IWindowManager windowManager)
        {
            this._view = view;
            this._windowManager = windowManager;
        }
    
        public void Close()
        {
            _windowManager.removeWindow(_view);
            _view.Close();
        }
    }
    
  4. Finally, I instantiate my WindowFactory and WindowManager in my MainWindow (Main, startup/default View) code-behind:

    public MainWindow()
    {
        InitializeComponent();
    
        var windowManager = new WindowManager();
        var windowFactory = new WindowFactory(windowManager);
        MenuItemsStackPanel.DataContext = new SideMenuViewModel(windowFactory, windowManager);
    }
    

I'm not including the Interfaces, since they contain simply the mocks of the used functions, nothing else.

I would really appreciate any feedback on my Factory design especially, since I might have made mistakes that I am unaware of, OR there could be a much better, cleaner way to achieve this.

\$\endgroup\$
1
\$\begingroup\$

Disclaimer: I haven't really used MVVM though am familiar with the pattern.


WindowFactory

The layers in MVVM, from upper to lower in terms of direction of dependencies (dependencies flow down), are View, ViewModel, and Model. Therefore it seems wrong to me that the ViewModel contains a View (by way of containing a ViewAdapter).

Another issue is that CreatePhoneWindow() actually displays the window. I would expect a factory to return a window, which the client can then display (or not) as and when desired. This isn't to say it is wrong, but it feels wrong (to me).

WindowManager

if (_windows.Contains(window) == false) is unnatural; use if (!_windows.Contains(window)) instead. Same for the comparison against true in the other method.

if (_windows.Count == 0) { return true; } 
else { return false; }

This also can be simplified to return _windows.Count == 0;. (But is it correct that when the _windows collection is empty it means windows are open?)

Also why are you using an ObservableCollection? It appears you're not binding to it, being as it's a private field, so why not just use a standard List<Window> instead?

ViewAdapter

Its purpose is to handle View close commands, that are coming from inside the View's ViewModel...

I don't understand why this class exists. If a ViewModel wants to say "time to close", it can raise an event. The View (Window) - which should have a dependency on the ViewModel - can subscribe to this event, and close itself. No need for an extra class to do its own closing. If you still want to have a WindowManager, then the WindowManager can subscribe to the Window's OnClose event and remove that object from its collection.

\$\endgroup\$
  • \$\begingroup\$ Let's reassume: Is your idea to use WindowFactory for creating windows without displaying them and for ex. fire an event that the window was created, that event being subscribed my WindowManager that will manage showing and hiding the windows, and also keep track of how many there are, and will subscribe to events fired by ViewModels? \$\endgroup\$ – Kamil Solecki Dec 6 '16 at 12:19
  • \$\begingroup\$ @KamilSolecki No I would expect a factory to create something and return what it created to the caller. I don't expect it to return void. This is aside from the fact that I don't personally see a use case for the WindowManager. What I would normally expect is for a View to create and "contain" a ViewModel (or be created and associated with a new/existing VM) and subscribe to any of the VM's events it wants to know about, and handle its own close. I don't see where a WindowManager enters into the picture from a MVVM perspective. \$\endgroup\$ – 404 Dec 6 '16 at 13:10
  • \$\begingroup\$ I see, but returning a View to a caller of the factory (ViewModel in my case) would be a violation of MVVM. Or should I introduce yet another level of indirection? \$\endgroup\$ – Kamil Solecki Dec 6 '16 at 20:11
  • \$\begingroup\$ @KamilSolecki No, the problem there is the ViewModel should absolutely not be creating a view, whether directly or via a factory. Your program starts up, creates an instance of a View, and the View creates an instance of a ViewModel. Or the program (think Composition Root in DI) creates a ViewModel, then creates a View and injects the ViewModel into the View. Or other ways, of course. The point is the ViewModel does not initiate construction of the View, it only knows about objects on its own or lower levels. The View is above it. The ViewModel should not have a factory to create Views. \$\endgroup\$ – 404 Dec 7 '16 at 0:43
  • \$\begingroup\$ @KamilSolecki The ViewModel should concern itself with exposing business logic and functionality in a UI-friendly way, but it should not know anything about the UI, much less create them directly or indirectly. It just provides a layer that the UI can attach itself to. \$\endgroup\$ – 404 Dec 7 '16 at 0:47
0
\$\begingroup\$

I think your mixing concerns here. If you are working in WPF you can declare the viewmodel in xaml. This is the absolute simplest way but runs out of steam fast.

A better way is to use a dependency injection container to wire up all the dependencies. A dependency injection container is effectively a factory.

You'll probably end up with a ViewModelBase. Put ICommands here and use binding from your view to the Icommand. Put virtual functions or callbacks from the viewmodelbase into the derived viewmodel to take on the close activities for that particular view/feature.

The requesting of a view/registration is a higher level feature, create a separate class for this. So the View has no reference to the over-all view manager. The view has no reference outside of bindings to the viewmodel, and the viewmodel has no reference to either the view or the view manager.

\$\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.