http://blogs.clariusconsulting.net/kzu

Daniel Cazzulino's Blog

Go Back to
kzu′s Latest post

You don’t need an IoC container or ServiceLocator for everything

Say you have a class that needs to collaborate with another, say a repository:

public class OrderProcessor
{
    public void ProcessPayment(Guid orderId, Payment payment)
    {
        using (var repo = new OrmRepository())
        {
            var order = repo.Find<Order>(orderId);
            order.Pay(payment);

            repo.Save(order);
        }
    }
}

Now that clearly is very hard to test ‘cause it’s directly instantiating the repository. So we know we have to refactor that and pass the repository instead, so that tests can replace the implementation and make assertions about the interaction (if we want to):

public class OrderProcessor
{
    private OrmRepository repository;

    public OrderProcessor(OrmRepository repository)
    {
        this.repository = repository;
    }

    public void ProcessPayment(Guid orderId, Payment payment)
    {
        var order = this.repository.Find<Order>(orderId);

        order.Pay(payment);

        this.repository.Save(order);
    }
}

Now we realize that for unit tests to actually go against the actual real repository (i.e. a database) would be pretty slow and inefficient: we’d have to drop/clean the DB for every test run, etc. Unnecessary complication (not that you don’t need to have *integration* tests that DO run against the real DB, though). So we go ahead and make an interface out of the dependency (the repository in this case), so that we can completely replace its implementation in the tests:

public class OrderProcessor
{
    private IRepository repository;

    public OrderProcessor(IRepository repository)
    {
        this.repository = repository;
    }

    public void ProcessPayment(Guid orderId, Payment payment)
    {
        var order = this.repository.Find<Order>(orderId);

        order.Pay(payment);

        this.repository.Save(order);
    }
}

Let’s not forget WHY we did all this: just so the behavior we’re creating can be easily tested, that’s all. It was never our goal to dynamically replace the repository, make it configurable externally or whatever. That’s NOT the point of the refactoring.

So what do you do when the need comes along to use some runtime framework (i.e. ASP.NET MVC, WCF, etc.) that by default expect your component to have a default parameterless constructor? (say this processor is an MVC controller or a web service). Well, it’s pretty damn easy, you just add a default ctor with the default implementation you know is the only one you have and will use at runtime!

public class OrderProcessor
{
    private IRepository repository;

    public OrderProcessor()
        : this(new OrmRepository())
    {
    }

    public OrderProcessor(IRepository repository)
    {
        this.repository = repository;
    }

    public void ProcessPayment(Guid orderId, Payment payment)
    {
        var order = this.repository.Find<Order>(orderId);

        order.Pay(payment);

        this.repository.Save(order);
    }
}

And that’s IT. What? You thought at this point you NEEDED to choose an IoC container from the myriad available, hook up a controller/instance factory on the runtime MVC/WCF, etc.?? NO, you DON’T.

An IoC container WILL help if you have complex dependency graphs to instantiate (in your default constructor) or you have truly pluggable components (i.e. you want to allow a component to be picked up automatically at runtime from some binary if it’s in a folder, etc.).

If you don’t, it’s perfectly fine to have a default constructor that is used at runtime and has a hardcoded instantiation of a dependency. It was never a requirement to make that thing configurable or dynamic. So don’t invent business requirements just ‘cause using an IoC container is fancier. As far as I’m concerned, you didn’t violate any sacred principle by having that default constructor in there.

 

If you were doing TDD (which you should) the process would be slightly different. Instead of refactoring to make it testable, you’d have the constructor receiving the IRepository right from the beginning. But the end result would be the same: just add a default ctor for the runtime framework to use and move on.

 

That said, an IoC container and a mocking framework are my first two choices when doing File | New Project Smile. I know I’ll need them for anything non-trivial.

Comments

13 Comments

  1. I would prefer property-injection in this case

    • That’s how I’ve been doing it; property injection.
      For misbegotten unit tests, as are described here, I assign in a fake repository or service into the property which will in yield a new real instance if accessed when value is null. In production, this property is not injected…

  2. The next thing that happens is that Joe Junior Developer comes by and wants to create an instance of OrderProcessor. Since he doesn’t know how to get an instance of IRepository, he’ll just use the default constructor instead.

    After all, the contract of the class is that this is a perfectly valid way to create an instance.

    This may work well for a few iterations, but a couple of years down the line, you’ll end up with a tightly coupled mess of Spaghetti Code. There’s a reason this is called the Bastard Injection (anti-)pattern.

    Remove the default constructor, and you still don’t need a DI Container – you just need a third party to connect the collaborators.

    • And why would Joe Junior Developer instantiate classes that are only instantiated by runtimes? (MVC or WCF?)

      Sounds more like an excuse to use a DI container ;) (or write more code for that “third party”, more stuff to explain to Joe).

  3. “Let’s not forget WHY we did all this: just so the behavior we’re creating can be easily tested”

    That may be why *you* did this. But it’s not why I would have done that same thing. I would have done it because depending on concrete dependencies changes the role of the processor object. It’s no longer simply an order processor that stores things. It’s an order processor that stores things to the database. It’s role has become both larger and more restricted. And worst of all, it’s done so *implicitly*. All of which means that it will be harder to maintain this class and keep its logic clean and isolated going forward through the life of the project.

    • How? All its code (except for that default ctor) only knows about IRepository.

    • “All its code (except for that default ctor) only knows about IRepository”

      The default ctor is part of the code, isn’t it? And if it’s public, there’s nothing to stop someone from using it. Especially if they are, as you described, “Joe Junior Developer who doesn’t understand what DI means”. The decision to offer a public default ctor with concrete dependencies creates an affordance to developers, especially those under time pressures. Unless those developers are mature and diligent (and you’ve already described the maintainer as someone who probably isn’t), they are going to take the easy path.

      No one likes to talk in these soft terms like “forces”, “pressure”, and “affordance” because they feel ephemeral in the moment. But when you look at how software evolves over time it’s hard to deny. Decisions like these are like changing the nutrient gradient in a petri dish. They have a subtle affect at any given moment which nonetheless dramatically affects the shape of what evolves over time.

  4. Hey Daniel!!

    In your first example you have:

    using (var repo = new OrmRepository())

    So, the repository is disposed at the completion of the action method.

    The two dependency-injected versions fail to dispose the repository, so you’re probably putting pressure on the finalizer as well as hanging on to connections longer than you need to ;)

    Once you start down the “DI” path, even with the simple poor-man’s approach, you eventually hit the realizations that call for some kind of automated assistance (i.e. an IoC container).

    I’m not arguing against your point though – you truly don’t need an IoC container in a simple example like this. I’d choose the first version (so clean and readable that it barely needs testing anyway).

    It seems to me that the “in between” versions suffer from greater complexity/fragility, without fully realizing the benefits that the technique of programming to abstractions can offer.

    Nick

    • Sure thing, I didn’t want to complicate things by passing in a Func instead ;) . Also, in this case (MVC/WCF) both can handle disposing the instance when they are done with it, so it would be just a matter of implementing IDisposable on it, I guess.

      Again, this is just to show that Joe Junior Developer can still get his work done even if he doesn’t understand what DI means. It’s a progression. For stupidly simple examples like this (and I’d say this is quite common), you don’t need to burden them by saying “you gotta learn DI/IoC before you create a MVC controller!”. Which seems to be the default for many.

      Ditto for mocking: you can write effective unit tests without knowing what mocking means (even if you do know how to create a manual ‘substitute’ for the IRepository here for testing purposes).

  5. This is a typical “poor’s man injection”.

    I actually agree with you. I have gone through all of these cases and concur that sometimes container is an overkill.

  6. I like to use this approach – I call it “the Default Pattern”.

  7. Nice job kzu explaing this simply.

    I agree many of the Joe Junior Developers out there will have completely overlooked DI/IoC approaches becuase of their perceived complexity, and unrealized value (of that aproach) in their growing stages, so this – as you say quite rightly – offers a progression for them that at least gives them the ability to mock and test their stuff, which is a huge step in the right direction of mastering their trade.
    A simple pattern that does have its weaknesses (veterans all know that) but is ultimately better than new’ing up the dependency which is all too often what is really found in codebases everywhere all over the world, written by folks who are always under time pressure, in a rush to get more stuff done rather than focusing on long term maintainability of their stuff.

    I have one ask though. How would you achieve the same outcome if your OrderProcessor class was static?, (i.e. only contained classic static methods (not extension methods)?

    Why do I ask? there are some ‘business’ layers that are immediately required to be implmented as static methods (i.e. from InvokeMethod() in WCF/WF workflows) that effectively can implement the logic layer and may also call directly a repository layer, in the same vein as your example.
    Of course you can get the static method to new up a instance of another class to use this instancing pattern, but why waste extra classes for just routing. Can this simple pattern be implemented ‘elegantly’ you think for a static OrderProcessor class you think?

  8. Simple,

    Make the default constructor protected, or make it a protected property, and build a proxy for testing. That will keep Joe Junior from doing somethings stupid and still allow you to inject your mock / stub for the test.