Is this correct use of Dependency Injection?

Question

I have a service layer that retrieves a DTO from a repository. Depending on a property in that DTO I need to use one of two strategies to perform calculations on the DTO. I have created a factory to return the appropriate strategy and I'm using a DI container (Munq) to instantiate the object.

public class CalculationFactory
{
    private readonly IDependencyResolver _resolver;
    public CalculationFactory(IDependencyResolver resolver)
    {
        ThrowIfNullArgument(resolver, "resolver", typeof(IDependencyResolver));
        _resolver = resolver;
    }

    public static ICalculation CreateCalculator(int serviceType)
    {
        switch (serviceType)
        {
            case 1: return _resolver.Resolve<ICalculation>("Type1");
            case 2: return _resolver.Resolve<ICalculation>("Type2");
            default: return _resolver.Resolve<ICalculation>("InvalidType");
        }
    }
}

This requires me to pass in the dependency resolver when I instantiate the factory so that it can be used to resolve/instantiate the appropriate calculation to use. Is this the correct approach? If I want to add a new type of calculation then I would need to update the factory CreateCalculator method and register the new type.

Update (long) I'm not really getting traction on the suggestions I'm afraid. I've gone back to my copy of Mark's DI in .Net and specifically chapter 6 on refactorings. So I have a class MeterReadings and I need to calculate a charge using one of two calculations based on a runtime value. I add an abstract factory to the mix but in my concrete factory I still need to new up the appropriate calculation. I feel like I'm missing the point here:

public enum ServiceType
{
    Actuals = 1, CopyBlock,
}

public interface IChargeCalculator
{
    decimal CalculateCharge();
}

public interface IChargeCalculatorFactory
{
    IChargeCalculator CreateChargeCalculator(ServiceType serviceType);
}

public class MeterReading
{
    private readonly IChargeCalculatorFactory chargeCalculatorFactory;
    public MeterReading(IChargeCalculatorFactory chargeCalculatorFactory)
    {
        if (chargeCalculatorFactory == null)
            throw new ArgumentNullException("chargeCalculatorFactory");
        this.chargeCalculatorFactory = chargeCalculatorFactory;    
    }
}

public class ConcreteChargeCalculatorFactory : IChargeCalculatorFactory
{
    public IChargeCalculator CreateChargeCalculator(ServiceType serviceType)
    {
        switch (serviceType)
        {
            case ServiceType.Actuals : return new ActualsChargeCalculator();
            default: return new CopyBlockChargeCalculator();
        }
    }
}

But in my container I can register the different calculators and if I pass in the container as the concrete factory I get something like the following (not tested) which frankly looks fairly reasonable to me. Any feedback/clarification would be welcome.

Container.Register<IChargeCalculator>("Actuals",
                                      c => new ActualsChargeCalculator());
Container.Register<IChargeCalculator>("CopyBlock",
                                      c => new CopyBlockChargeCalculator());
...
public enum ServiceType
{
    Actuals = 1, CopyBlock,
}

public interface IChargeCalculator
{
    decimal CalculateCharge();
}

public class MeterReading
{
    private readonly IDependencyResolver chargeCalculatorFactory;
    private ServiceType serviceType;
    public MeterReading(IDependencyResolver chargeCalculatorFactory)
    {
        if (chargeCalculatorFactory == null)
            throw new ArgumentNullException("chargeCalculatorFactory");
        this.chargeCalculatorFactory = chargeCalculatorFactory;    
    }

    public decimal Charge
    {
        get
        {
            return chargeCalculatorFactory.Resolve<IChargeCalculator>(serviceType.ToString());
        }
    }
}

Side note, the way you are resolving your dependencies here is typically referred to as a 'Service Locator' pattern (or anti-pattern, depending on your point of view): http://blog.ploeh.dk/2010/02/03/ServiceLocatorIsAnAntiPattern.aspx I usually try to avoid the Service Locator pattern, but sometimes it is unavoidable. — CodingWithSpike, Dec 22 '11 at 01:56
Thanks rally25rs, I had a look at Mark's post but it doesn't provide a way to refactor away from Service Locator to something better. — David Clarke, Dec 22 '11 at 02:04
Here's how: http://stackoverflow.com/questions/1943576/is-there-a-pattern-for-initializing-objects-created-via-a-di-container/1945023#1945023 — Mark Seemann, Dec 22 '11 at 07:17

score 3 · Accepted Answer · answered Dec 22 '11 at 01:39

Yes, I would say you are doing it in a proper way.

Though you write that you need to "pass in the dependency resolver ", is there a reason that you can't have the factory class injected into the class where it is needed? Then the factorys dependency on the dependency resolver should be resolved by the dependency resolver itself (to itself).

I hope that sentence made sense.

I have tried to come up with a "cleaner" solution, but have not found any yet. A solution similar to what is suggested in the question (which is not exactly the same) Dessus links is certainly possible, but I really see that as just moving the same code to another place. Remo also writes that it could be done in a factory class instead of a factory method.

It's a bit like deciding between writing a lampda or a helper class. It mostly comes down to readabilty, and for me a factory method is just to big, and having it in a module initializer or bootstrapper seams to disorganized.

BTW I can recommend Ninject, it is a really good, clean DI. Plus it's documentation uses Ninja examples ;)

I second the recommendation for Ninject. Beware the pirates, though. — , Dec 22 '11 at 01:51
Can you give me an example usage of injecting the factory class? If I inject the factory, how do I resolve the calculations in the Create method without breaking the object graph, ie if the calculations need to resolve other objects. I can't just _new_ up the appropriate object. Does that make sense? Thanks for the recommendation for Ninject, I've been happy with Munq to date - it's relatively light and fast. — David Clarke, Dec 22 '11 at 01:53

score 0 · Answer 2 · edited May 23 '17 at 12:27

I believe this question has been answered here, where it is suggested you use meta data against your bindings and create bindings for each implementation. You then use a binding to pull the meta data to choose which binding to use: Ninject conditional binding based on property value . Possibly this may or may not be possible with Munq? Not sure.

The person (Remo) who answered that question is one of the architects of ninject, and is very knowledgeable. I believe that his answer should hold alot of weight. (I am a basing this of being a subscriber to the ninject mailing list, and seeing him answer about half of all the questions).

Is this correct use of Dependency Injection?

2 Answers2