Merits of DI, Abstract Factory implementation review

Question

I'm writing a brand new system and trying to stay true to the SOLID principles, specifically Interfacing and Dependency Injection. We are sticking closely to the Microsoft stack so we are using C# and the Unity container. I am in a relatively immature shop as far as SOLID concepts go so I try to keep things as simple as possible for everyone, including myself.

I have a style in which I prefer to use the constructor to accept runtime parameters and validate all the fields the object might need up front. This makes simple constructor-based injection difficult and led me down the abstract factory path.

I also have a dependency in which I need a list of Interfaces. Again this was not something that Unity elegantly supported and again the abstract factory seemed right.

http://blog.ploeh.dk/2010/11/01/PatternRecognitionAbstractFactoryorServiceLocator/ http://blog.ploeh.dk/2012/03/15/ImplementinganAbstractFactory/

I started out with lofty ideas of a generic abstract factory but to be honest I was having a difficult time figuring out how to get the container registrations right. If it was hard for me it was going to be that much harder to explain it and sell it to my team.

So I stepped back and came up with this pattern where I have an Interface and corresponding Factory Interface for each of my dependencies. The container registration becomes super simple.

_container.RegisterType<IChartFactory, ChartFactory>();
_container.RegisterType<IDocumentFactory, DocumentFactory>();
_container.RegisterType<IReportFactory, ReportFactory>();
_reportFactory = _container.Resolve<IReportFactory>();
_report = _reportFactory.CreateReport1(runTimeData);

If we ever need to change out a dependency we implement the underlying interface with the minimal added step of implementing the corresponding factory as well.

public class DocumentFactory : IDocumentFactory
{  
    public IDocument Create(string filePath)
    {
        return new Document(filePath);
    }
}

public interface IReportFactory
{
    IReport CreateReport1(object runTimeData);
    IReport CreateReport2(object runTimeData);
}

public class ReportFactory : IReportFactory
{
    IDocumentFactory _documentFactory;
    IChartFactory _chartFactory;

    public ReportFactory(IDocumentFactory documentFactory, IChartFactory chartFactory)
    {
        _documentFactory = documentFactory;
        _chartFactory = chartFactory;
    }

    public IReport CreateReport1(object runTimeData)
    {
        return new ConcreteReport1(runTimeData, _documentFactory, _chartFactory);
    }

    public IReport CreateReport2(object runTimeData)
    {
        return new ConcreteReport2(runTimeData, _documentFactory, _chartFactory);
    }
}

I do find it simple to understand and explain. If the dependency changes, implementing a few lines of code for the factory seems like a small price to pay as opposed to modifying every spot in the system where the type gets newed up.

More dependencies are creeping in and all these interfaces and factories are feeling like overkill. I'm worried about pushback. I'm not sure the merits are justifying the pattern and I'm not sure if I'm doing it right. Any thoughts are greatly appreciated!

Answer 1

Welcome to the fun world of IoC. I went through this recently myself. It can be a massive change and somewhat frustrating.

A couple items of feedback:

1. Don't be too concerned that you have too many interfaces and factories. It is a shock but this is the way it is commonly done. It was an adjustment for me, too.

2. If the developers on your team aren't used to IoC, they may get a little frustrated debugging code, as they can no longer use "Go to definition" to trace where a call goes (it will just show them the interface and not the implementation). To make this a little less painful, I recommend putting your composition root in a consistent place (e.g. always in global.asax.cs), use a consistent naming convention (e.g. InitContainers or something), and make sure your developers know to look there.

3. I'm a little concerned about this comment:

I have a style in which I prefer to use the constructor to accept runtime parameters and validate all the fields the object might need up front.

This is another point where you need to adjust your mindset. Constructors should do nothing (other than accept DI). This is because the ctor method is notoriously difficult to unit test in isolation. If you absolutely have to pass parameters in the constructor, just store them in member variables and defer using them for anything using lazy initialization.

4. Don't forget, for factories, you don't want to create a new one each time. With Unity I believe you would use either RegisterInstance or a Lifetime manager to control whether Resolve<> will return a new instance each time or the same instance per process/thread/request.