1

I'm pretty new to DI and i have a few questions i hoped people could clear up for me, I'm currently working on a WPF-MVVM system that uses Caliburn Micro, using a MEF container.

This application is used to keep track of shipments and has several parts. I hope i can explain this clearly enough, but please do point out if it is't clear.

I have several entities returned from the database (via webservices) examples are shipments, containers, packages.

for each of these entities i have a model which wraps the webservice entity, and a manager, the manager is responsible for the standard CRUD operations via webservices, as well as storing the ObservableCollection of the model, these managers are then injected into the constructors of viewmodels that require access to these lists.

So, i have shipment > shipmentManager > shipmentListViewModel, this was done to allow multiple viewmodels work with the same list of shipments

However, I've started to run into issues where some viewmodels have constructors with 6+ managers included, and some cases that are only used to pass onto newly constructed dialog viewmodels.

I'm hoping that someone could suggest a clean solution to this issue, I'm thinking of a single class which will become a container for all managers, and then i can simply inject that container class and use that to get the desired Manager, however I've seen people advising against that method, without clearly stating why.

Also, one other question, my Models implement IEditableObject and because my managers are responsible for maintaining the list of models, as well as saving changes to those models, would publishing an event inside EndEdit that the manager picks up be an issue?

EDIT: Code as requested:

The bootstrapper create and exports the required classes:

        protected override void Configure()
    {
        container = new CompositionContainer(new AggregateCatalog(AssemblySource.Instance.Select(x => new AssemblyCatalog(x)).OfType<ComposablePartCatalog>()));

        CompositionBatch batch = new CompositionBatch();
        IEventAggregator eventAggregator = new EventAggregator();
        batch.AddExportedValue<IWindowManager>(new WindowManager());
        batch.AddExportedValue<IEventAggregator>(eventAggregator);
        batch.AddExportedValue<IManager<ShipmentContainer>>(new ContainerManager());
        batch.AddExportedValue<IManager<Item>>(new ItemManager());
        batch.AddExportedValue<IManager<OrderedItem>>(new OrderedItemManager());
        batch.AddExportedValue<IManager<Package>>(new PackageManager());
        batch.AddExportedValue<IManager<Proforma>>(new ProformaManager(eventAggregator));
        batch.AddExportedValue<IManager<Project>>(new ProjectManager());
        batch.AddExportedValue<IManager<Shipment>>(new ShipmentManager(eventAggregator));
        batch.AddExportedValue<IManager<PackingItem>>(new PackingListManager(eventAggregator));
        batch.AddExportedValue(container);

        container.Compose(batch);
    }

ContentViewModel handled the menu clicks, which allows for the opening of several diaglogs, the constructor contains a large number of DI:

    public LBLContentViewModel(IWindowManager windowManager, IManager<Project> pManager, IEventAggregator eventManager, IManager<Item> iManager, IManager<PackingItem> plManager, IManager<Shipment> sManager)
        {
          ...
        }

and dialogs are shows as follows:

 public void OpenProject()
    {
        ProjectSearchViewModel viewModel = new ProjectSearchViewModel(_eventAggregator, _projectManager);
        this._windowManager.ShowDialog(viewModel);
    }

Hopefully this is the code you wanted to see charleh, if not please let me know and i'll try and provide what is needed.

Ben
  • 1,291
  • 2
  • 15
  • 36
  • 1
    Ok - so you need to get an instance of the `ProjectSearchViewModel` - why not just use `IoC.Get()` (your VMs should also be exported) - then move your `IEventAggregator` and `IManager` into the constructor of `ProjectSearchViewModel`. What you are doing here is not IoC, since the control of dependencies lies with a bloated class – Charleh Sep 04 '13 at 10:05
  • I think that's what margabit was suggesting, I plan to do this now, i was a little concerned about the number of exports inside the bootstrapper but i take it this is not an issue? – Ben Sep 04 '13 at 10:07
  • A lot of IoC containers allow you to mass register dependencies based on convention - not sure what MEF lets you do, but explicitly exporting all objects one by one is normal (the mass registration was invented to make things a little less wordy in these registration classes, that is all) – Charleh Sep 04 '13 at 10:08
  • excellent, thank you once again for your help charleh. I was a little concerned about showing that code as i felt i was doing something very wrong, thankfully it seems not. – Ben Sep 04 '13 at 10:10
  • An example - in Windsor you would use `_container.Register(Classes.FromThisAssembly().InSameNamespaceAs())` rather than registering absolutely everything explicitly. MEF appears to support conventions, but I'd have to do some reading as to what it exactly supports – Charleh Sep 04 '13 at 10:11
  • I shall look into that, could you edit your answer with the information in these comments so i can mark it as accepted? – Ben Sep 04 '13 at 10:20

4 Answers4

3

Two comments on this. Might not be the answer that you are looking for, but maybe it can ring a bell.

1.- I would not use a Manager object. Manager is not a good word, it can mean anything and it is probably going to be responsible of many things, definately not a good thing because it would break the Single Responsability Principle (SRP), one of the SOLID principles.

2.- The same way a Manager with multiple responsabilities might not be a good approach, a class that has 6 dependencies makes me think that this class is doing too much.

Obviously you could fix this problem using Dependency Injection and forgetting about the pain of creating new objects every time. However, I think it will only be patch to a minor problem but not for the main problem.

My suggestion is that you analize the class and its many dependencies and try to split things up in order to create units with less responsability applying some Object Oriented Principles.

Sometimes our classes grow and grow and look like anything can't be split, specially with ViewModels or Controllers. However, a View can have multiple controls and multiple ViewModels.. maybe this is the way to go.

By the way, this question can be helpful too!

AFTER YOUR EDIT:

I would do as we spoke in the comments:

Register the Dialog first..

batch.AddExportedValue<ProjectSearchViewModel>(new ProjectSearchViewModel(eventAggregator,projectManager));

In the main ViewModel you can have then:

 public LBLContentViewModel(ProjectSearchViewModel projectSearchViewModel, OtherDependencies ...)
 {
      ...
      _projectSearchViewModel = projectSearchViewModel;
  }

Open the Dialog:

public void OpenProject()
{
    this._windowManager.ShowDialog(_projectSearchViewModel);
}

This way you remove Dependencies from the MainViewModel and move them where they really belong, to the Dialog.

In my opinion MEF is powerful library to use in large applications and be able to use many assemblies without coupling them. It can also be used as a Dependency Injection engine, however I think it was not designed for this purpose.

Take a look at this great post about it. I would suggest adding a IoC library such as Unity or Autofac to do Dependency Injection more effectively.

Community
  • 1
  • 1
margabit
  • 2,924
  • 18
  • 24
  • Thanks for the reply, I only recently came across the idea of "SOLID", and working continually shows me how lacking my university education actually was! However, i "think" that my managers are only doing on thing, they manage the ObservableCollections > CRUD operations. Unless i was to split the manager into a its basic functions (save, update etc.) i cant see how i break SRP (I could be wrong however) Also the viewmodel that recieves 6 injections is simply because it creates dialogs that need those passing. – Ben Sep 04 '13 at 09:06
  • What about trying to isolate the Dialogs ViewModels, so you can inject them in your main ViewModel and the injector resolves the Dialog dependencies? – margabit Sep 04 '13 at 09:14
  • I have considered doing this, but wouldn't that just shift the problem into the bootstrapper, having to create and export all dialog viewmodels in there instead? – Ben Sep 04 '13 at 09:31
  • 1
    The bootstrapper would only inject the dependencies to the object that is requested. It's the same action that the main ViewModel is doing now. But if the ViewModel is the responsible to create the Dialogs, everytime you need a new object in a Dialog, the main ViewModel will need to get it first. This means that your main ViewModel is full of dependencies that does not even belong to it.. I think DI could make things cleaner in this case – margabit Sep 04 '13 at 09:36
  • Ah i see your point, i guess keeping unused objects out of the viewmodel would certainly be the way to go. Sorry if i'm a little slow on the uptake I'm trying to learn a whole set of new concepts during production, it's a little overwhelming at times haha. Thanks again. – Ben Sep 04 '13 at 09:40
2

I believe the best answer is already in your question:

I'm hoping that someone could suggest a clean solution to this issue, I'm thinking of a single class which will become a container for all managers, and then i can simply inject that container class and use that to get the desired Manager, however I've seen people advising against that method, without clearly stating why.

This is the best answer in my opinion. If you find that you're constantly injecting the same set of things into one of your constructors, then most likely that set of objects forms some sort of logical grouping that would make sense if combined into a class.

Also take what @margabit said in his answer about analyzing classes and their dependencies. It's possible that a different structure to your design might have avoided this in the first place, but its not always possible to go back and change everything.

Steve
  • 6,334
  • 4
  • 39
  • 67
2

I'm making a few assumptions here but:

Is there something preventing you from having the dialogs specify their dependencies and have those dependencies resolved by the container?

It sounds like your manager classes would be singleton lifestyle (correct me if I'm wrong); if this is the case, the VMs which cause dialogs to be shown don't need to take a dependency for managers they aren't really interested in: the dialog itself should resolve these dependencies upon instantiation (once again my assumption is that your dialogs would be transient)

SRP states that objects should have a single responsibility; whilst it sounds like making a class that's only responsible for containing more classes is a single responsibility, you are essentially just creating another container, something that the IoC container is already doing.

Could you post some example code or clarify how you have your IoC setup - what is singleton, what is transient? Most IoC containers also have factory options (such as Castle Windsors TypedFactory) which give you more control over transient instantiation so this might be an option (depends on your complexity)

Edit: Ok after seeing your code the solution is simple enough...

If you aren't already doing so, export your ViewModels in MEF.

The container should be able to resolve all your VMs as components too. Don't worry about the number of exports. I'm not sure if MEF supports convention based registration (quick search suggests it does but to what degree I'm not sure)

Once you've exported your VMs, you can get MEF to instantiate and satisfy dependencies when the VM is required

When you need an instance of the VM you can use the static IoC class provided in CM (it just resolves from the container)

var vm = IoC.Get<ProjectSearchViewModel>()

and now show this VM in a dialog

_windowManager.ShowDialog(vm);

You could also go one better and move the dependency for the ProjectSearchViewModel to the constructor of the main VM so it's more clear cut (but either way will work)

Since dependencies are now being satisfied upon instantiation, the dialog can specify what it is dependent on without the parent window needing to know

The LBLContentViewModel constructor then becomes much less bloated:

 public LBLContentViewModel(IWindowManager windowManager, IEventAggregator eventManager)
 {
     ...
 }

And ProjectSearchViewModel correctly specifies it's dependencies

public ProjectSearchViewModel(IEventAggregator eventAggregator, IManager<Project> projectManager)
{
    ...
}

And this is the Inversion part of IoC - now components and subcomponents specify what they require and the container provides. Previously it was the other way round with a component deciding what the subcomponents require...which would have been painful going forward!

Charleh
  • 13,749
  • 3
  • 37
  • 57
  • I'll try posting some code now, hopefully i can clear up your questions i must admit to having to look up what transient means in this respect, however i should be able to show what you need. – Ben Sep 04 '13 at 09:48
  • Transient just means more than one instance - the container serves a new instance each time a service is required (most containers have additional options in order to satisfy variable run time-dependencies) – Charleh Sep 04 '13 at 09:50
-2

I would recommend using any DI (Dependency Injection) component. It is very clean and powerful.

Some popular DI components : - Ninject - Unity - Castle.Windsor - Autofac - StructureMap

Otherwise you can define a property whose type is the interface type for which you have two or more classes. Then create the appropriate object dynamically and set it to the property. You can store the class name and fully qualified class name mapping in config file which will be used while creating appropriate object dynamically.

Adarsh Kumar
  • 1,134
  • 7
  • 21
  • I'm not really sure what your suggesting here, I'm already using DI to inject the managers into the viewmodels, I'm looking for a way to reduce the number of injections required in those viewmodels. – Ben Sep 03 '13 at 16:21