Reputation: 292425
I know that injecting an IoC container as a dependency is usually considered a bad thing, because it basically the same as the Service Locator anti-pattern. However there is a case where I think it might make sense, especially since I don't see an easy way around it.
Consider these classes :
// domain class
public class Foo
{
}
// ViewModel
public class FooViewModel
{
[Dependency]
public IBar Bar { get; set; }
[Dependency]
public IBaz Baz { get; set; }
private readonly Foo _foo;
public ViewModel(Foo foo)
{
_foo = foo;
}
}
// Dependencies used by ViewModel
public interface IBar { void Frob(); }
public interface IBaz { void Fiddle(); }
Now suppose I have to display a list of FooViewModel
, built from a list of Foo
. To create an instance of FooViewModel
from a Foo
, I have the following choices:
just call the constructor and set the dependency manually:
var vm = new FooViewModel(foo) { Bar = _bar, Baz = _baz };
this is impractical, because:
FooViewModel
needs to have Bar
and Baz
as dependencies, even if it uses them only to create FooViewModel
FooViewModel
, I'll have to update the code everywhere I create an instance of it.Both issues could be solved by creating a factory, but that factory would need to be written and maintained. In this very basic scenario it wouldn't be an issue, but for a large real-world app, it can become quite cumbersome if there are many factories with many dependencies.
use the IoC container to resolve FooViewModel
:
var vm = _container.Resolve<FooViewModel>(new ParameterOverride("foo", foo));
This takes care of the dependencies, but then I have to use the container in my ViewModel layer, which is bad (or is it?).
I was thinking about a compromise: create a factory class that only has the container as a dependency, and uses it to build the FooViewModel
:
public class FooViewModelFactory
{
[Dependency]
public IUnityContainer Container { get; set; }
public FooViewModel CreateFooViewModel(Foo foo)
{
return Container.Resolve<FooViewModel>(new ParameterOverride("foo", foo));
}
}
This way I have a very simple factory that is easy to maintain, and the only classes that have the container as a dependency are the factories, not the actual business-related code.
Do you see any drawback in this approach? Am I missing a better/simpler solution?
Upvotes: 2
Views: 404
Reputation: 292425
I eventually adopted an approach similar to what I had shown in my question:
public class FooViewModelFactory
{
[Dependency]
public IUnityContainer Container { get; set; }
public FooViewModel CreateFooViewModel(Foo foo)
{
var vm = new FooViewModel(foo);
Container.BuildUp(vm);
return vm;
}
}
The use of BuildUp
makes the code more readable than using ParameterOverride
.
Using the container as a dependency might be a code smell, but in this case it definitely makes the code simpler, and I haven't noticed any drawback in a few months of using this approach. (I inject the container only in factories, though).
Ideally, the approach suggested by BatteryBackupUnit would have been better, but without runtime code generation, it's impractical, and generating the code at compile time is too much work for too little benefit, so I chose the more pragmatic approach.
Upvotes: 0
Reputation: 13233
I agree that service locator is an anti-pattern.
That said, the question you've posted can only be answered by yourself and does actually not fit the Q&A Format of Stackoverflow: It's highly opinion based. It might be a better fit on https://softwareengineering.stackexchange.com/
The important thing about deciding on this is evaluating and understandig all possible solutions and their side effects. Side effects may also include concerns about the people who are maintaining the code. Are they capable of understanding it? How do they work? Can you educate them?
So i gather the question you should actually ask is: how else can this problem be solved? Then try it out and see for yourself how the solutions compare.
Putting too much value on tips like "don't do that" (without further explanation and understanding) will only result in .. no growth! Which is basically the worst thing that can happen (not learning anything ever => always bad design).
For your specific question i would like to pitch in with a possible solution:
Use an auto-generated factory. This is very similar for the factory type you have suggested, but it limits your coding to the parameters you want to supply. All the others you don't have to care about. For example, Ninject features the Factory extensions where you can do stuff like:
public class FooViewModel
{
public FooViewModel(Parameter parameter, Bar bar, Nice nice) { ...}
}
public interface IFooViewModelFactory
{
FooViewModel Create(Parameter parameter);
}
kernel.Bind<IFooViewModelFactory>().ToFactory();
FooViewModel instance = kernel.Get<IFooViewModelFactory>()
.Create(new Parameter("my parameter"));
that's basically all of the code you got to maintain. Adding / changing / removing a dependency @ FooViewModel
does not result in a change of the factory (unless you do actually want to provide the parameter).
A drawback may be, that that part of the object graph - including the dependencies - is only instanciated when the factory .Create(..)
method is called. This is dependent on the way the auto-factory-generating code is implemented.
In ideal Dependency Injection the object graph is built at the composition root, at the start of the application, and everything is built at once. That way you'll detect IoC configuration issues right away when you start the application, not later on when you use a specific feature and click some button - which will call some factory.
As far as i know for unity there's some kind of extension that was ported from Castle.Windsor - TecX.Unity.Factories
, see http://outlawtrail.wordpress.com/2012/07/27/auto-generated-factories-with-unity/.
Then there's also http://www.nuget.org/packages/Unity.TypedFactories, which is basically the same.
You might even be content with Unity's "Automatic Factory" support (Func<>
). I actually don't know whether it's able to handle parameters, i'm quite new to unity myself.
Upvotes: 1
Reputation: 355
If all the view models can live with the same instance of IBar
and IBaz
, then you can simply define a factory that has those instances as dependencies:
public class FooViewModelFactory
{
private readonly IBar _bar;
private readonly IBaz _baz;
public FooViewModelFactory(IBar bar, IBaz baz)
{
_bar = bar;
_baz = baz;
}
public IEnumerable<FooViewModel> CreateViewModels(IEnumerable<IFoo> foos)
{
return foos.Select(f => new FooViewModel(f) {Bar = _bar, Baz = _baz});
}
}
I don't see anything bad about instantiating view models by hand since you have to pass different implementations of IFoo
anyway.
Upvotes: 0