How to deal with constructor over-injection in MVVM application

Question

I have been reading about the problem of constructor over-injection. It all makes sense that this is a sign where the SRP is not being followed correctly etc. (I am using Ninject by the way!)

However, I am having a hard time understanding how this can be solved in my case. The biggest problem is in my viewmodel where I am injecting DTO mappers and Repositories to be used with my properties.

Here is an example of what my viewmodel constructor could look like:

public MainViewModel(
        IGenericRepository<MainDbContext, Product> productRepository,
        IGenericRepository<MainDbContext, Person> personRepository,
        IGenericRepository<MainDbContext, Order> orderRepository,
        ProductMapper productMapper,
        PersonMapper personMapper,
        OrderMapper orderMapper,
        IViewModelLoader viewModelLoader, 
        IEventAggregator eventAggregator)
    {
        _productRepository = productRepository;
        _personRepository = personRepository;
        _orderRepository = orderRepository;
        _productMapper = productMapper;
        _personMapper = personMapper;
        _orderMapper = orderMapper;
        _viewModelLoader = viewModelLoader;
        _eventAggregator = eventAggregator;

        _eventAggregator.Subscribe(this);

    }

My guess is that I am not using the repositories/mappers correctly and they should be moved out of the viewmodel... I'm not sure exactly where or how. Which is the reason for my question.

The architecture of the application looks like this:

Company.Product.Core
Company.Product.DataAccess
Company.Product.Domain
Company.Product.Presentation

The GenericRepository is placed inside Company.Product.DataAccess.Repositories and mappers inside Company.Product.Domain.Mappers

Do you access Products, People, and Orders from a single window/view? — Yacoub Massad, Sep 23 '15 at 17:29
@YacoubMassad. The above repositories have been renamed to some examples. But my real constructor looks something like that and yes, they are all accessed in a single view.^ — Joakim Hansson, Sep 23 '15 at 17:30
Why do you inject into your constructor? Are you using some sort of IoC containers? — Lance, Sep 23 '15 at 17:31
Can't you split your view into multiple views, and then your ViewModel to multiple view models? — Yacoub Massad, Sep 23 '15 at 17:31
@LawrenceA.Contreras Sorry I should of mentioned that, I am using Ninject so yes. — Joakim Hansson, Sep 23 '15 at 17:32
@YacoubMassad Thanks for your quick reply! I could split it, the problem is that the real repositories is all related to eachothers which is why I am currently having them in the same view/viewmodel — Joakim Hansson, Sep 23 '15 at 17:33
I don't think it's a problem if you're using an Ioc. I myself would allow all those constructor if I have an Ioc to resolve it. It can also make my code more readable. Just by looking at the constructor, you can see all the dependencies of the class. Are you actually using those dependencies inside the MainViewModel class? — Lance, Sep 23 '15 at 17:34
I think that without more details about this View/ViewModel, it would be hard to answer your question. — Yacoub Massad, Sep 23 '15 at 17:37
@zatixiz side note on post edits: avoid "EDIT:"/"UPDATE:" if possible, try to make updates in such a way so post reads as on consistent pieces instead of collections of historical notes. — Alexei Levenkov, Sep 23 '15 at 17:39
@LawrenceA.Contreras Thank you Lawrence! I'm just afraid that the constructor injection will get even bigger and put too much responsibilities in it as I have to create a new IGenericRepository for every new table that the mainviewmodel interacts with. — Joakim Hansson, Sep 23 '15 at 17:42
To followup on @YacoubMassad upvoted comment, you can also compose ViewModels even w/o splitting the view (e.g MainViewModel(Person[Main]ViewModel, Product[Main]ViewModel...) ), inner ViewModels can be bound directly, and eventually reused elsewhere (though to be reused you usually need re-usable view components) — Gluck, Sep 23 '15 at 17:44
@YacoubMassad I will do my best to explain it a bit better. There is one main part of the view that gives the user an overview of 5+ tables, all related to eachothers. For each table I have to create a new IGenericRepository which will then be injected into the constructor. I am building a better base viewmodel so hopefully it can help solve the cluttering of the constructor. — Joakim Hansson, Sep 23 '15 at 17:44
@Gluck. Ahh okay, thanks for your comment! That makes sense. I guess the only reason why I've kept it in one viewmodel up to this point is because I am using Caliburn.micro and thought it would make it easier for me to keep it in one viewmodel for the convention. I will try to split it into different viewmodels and update the post with progress! — Joakim Hansson, Sep 23 '15 at 17:48
As long as you don't have one ViewModel that does everything, I don't see a problem here. — vidalsasoon, Sep 23 '15 at 18:49

score 4 · Accepted Answer · answered Sep 23 '15 at 19:47

Looking at the constructor list, it looks like things come in pairs:

productRepository/productMapper
personRepository/personMapper
orderRepository/orderMapper

This seems to indicate that MainViewModel composes something related to products, persons, and orders.

Could you instead model it so that it instead composes three other View Models? Hypothetical ProductViewModel, PersonViewModel, OrderViewModel classes..?

Does it even have to be exactly these three classes? Could MainViewModel instead compose any number of other View Models?

That would reduce the constructor to something like this:

public MainViewModel(
    IReadOnlyCollection<IViewModel> viewModels,
    IViewModelLoader viewModelLoader, 
    IEventAggregator eventAggregator)

which seems much more reasonable.

This would essentially be an implementation of the Composite pattern, which is often a natural fit for UI modelling - where it's often called Composite UIs.

Another thing that makes me wonder is what that IVewModelLoader does?

Fantastic answer Mark! You are right about repositories and mappers coming in pairs. This occurs for almost every repository I add, except for some exceptions where data does not have to be mapped. I'm glad you answered as I was actually reading your article about fascade services and the related answer you linked above. I'll give it a try implementing what you suggested here and let you know how it goes! Regarding the IViewModelLoader.. That's what's responsible for openening my views and validation(TODO atm haha). The class looks like this: http://pastebin.com/hbCEc41s — Joakim Hansson, Sep 23 '15 at 20:14

How to deal with constructor over-injection in MVVM application

1 Answers1