[SOLVED] Is it a good design to inject services as factories?

Issue

I have been reading Mark Seemann’s excellent book on DI and hope to implement it in my next WPF project. However I have a query regarding object lifetime. So far, most examples seem to explain the repository pattern per request for MVC applications. In WPF there isn’t really an alternative to this (I think). Seeing as the object graph of the entire application is constructed in the composition root, how can I make sure that my unit-of-work stuff is working properly. For example:

public class ContextFactory : IContextFactory
{
    DBContext context;

    public ContextFactory()
    {
        context = new MyDBContext();
    }

    public DBContext GetContext()
    {
        return context;
    }
}

public class ItemOneRepository() : IItemOneRepository
{
    DBContext context;

    public ItemOneRepository(IContextFactory contextFactory)
    {
        this.context = contextFactory.GetContext();
    }

    public IEnumerable GetItems()
    {
        return context.ItemOnes;
    }
}

public class ItemTwoRepository() : IItemTwoRepository
{
    DBContext context;

    public ItemTwoRepository(IContextFactory contextFactory)
    {
        this.context = contextFactory.GetContext();
    }

    public IEnumerable GetItemsByItemOneID(int itemOneID)
    {
        return context.ItemTwos.Where(i => i.itemOneID == itemOneID);
    }
}

public class ThingService : IThingService
{
    IItemOneRepository itemOneRepo;
    IItemTwoRepository itemTwoRepo;

    public ThingService(
        IItemOneRepository itemOneRepository,
        IItemTwoRepository itemTwoRepository)
    {
        itemOneRepo = itemOneRepository;
        itemTwoRepo = itemTwoRepository;
    }

    public IEnumerable Things GetThing()
    {
        var ItemOnes = itemOneRepo.GetItems();

        return ItemOnes.Select(i =>
            new Thing(
                i.FieldOne,
                i.FieldFour,
                itemRepoTwo.GetItemsByItemOneID(i.ID)
            )
        );
    }
}

In this case the MyDBContext instance is created through ContextFactory in the composition root. ItemOneRepository and ItemTwoRepository are using the same unit-of-work (MyDBContext), but so is the rest of the application which is plainly wrong. What if I changed the repositories to accept a DBContext instead of ContextFactory and added a ThingServiceFactory class like:

public ThingServiceFactory : IThingServiceFactory
{
    IContextFactory contextFactory;

    public ThingServiceFactory(IContextFactory factory)
    {
        contextFactory = factory;
    }

    public IThingService Create()
    {
        MyDBContext context = contextFactory.Create();
        ItemOneRepository itemOneRepo = new ItemOneRepository(context);
        ItemOneRepository itemTwoRepo = new ItemTwoRepository(context);
        return new ThingService(itemOneRepo, itemTwoRepo);
    }
}

This is better as I can now pass the ThingServiceFactory to my ViewModels instead of an instance of ThingService (complete with DBContext). I can then create a unit-of-work whenever I need one and instantly dispose of it when I’ve finished. However, is this really the correct approach. Do I really need to write a factory for every unit-of-work operation I need? Surely there is a better way…

Solution

There’s IMO only one good solution to this problem and that is to apply a command-based and query-based application design.

When you define a single ICommandHandler<TCommand> abstraction to define business transactions, you can inject closed versions of that interface into any form that needs this. Say for instance you have a "move customer" ‘command’ operation:

public class MoveCustomer
{
    public Guid CustomerId;
    public Address NewAddress;
}

And you can create a class that will be able to execute this command:

public class MoveCustomerHandler : ICommandHandler<MoveCustomer>
{
    private readonly DBContext context;

    // Here we simply inject the DbContext, not a factory.
    public MoveCustomerHandler(DbContext context)
    {
        this.context = context;
    }

    public void Handle(MoveCustomer command)
    {
        // write business transaction here.
    }
}

Now your WPF Windows class can depend on ICommandHandler<MoveCustomer> as follows:

public class MoveCustomerWindow : Window
{
    private readonly ICommandHandler<MoveCustomer> handler;

    public MoveCustomerWindows(ICommandHandler<MoveCustomer> handler)
    {
        this.handler = handler;
    }

    public void Button1Click(object sender, EventArgs e)
    {
        // Here we call the command handler and pass in a newly created command.
        this.handler.Handle(new MoveCustomer
        {
            CustomerId = this.CustomerDropDown.SelectedValue,
            NewAddress = this.AddressDropDown.SelectedValue,
        });
    }
}

Since MoveCustomerWindow lives for quite some time, it will drag on its dependencies for as long as it lives. If those dependencies shouldn’t live that long (for instance your DbContext) you will be in trouble and Mark Seemann calls this problem Captive Dependency.

But since we now have a single ICommandHandler<TCommand> abstraction between our presentation layer and our business layer, it becomes very easy to define a single decorator that allows postponing the creation of the real MoveCustomerHandler. For instance:

public class ScopedCommandHandlerProxy<TCommand> : ICommandHandler<TCommand>
{
    private readonly Func<ICommandHandler<TCommand>> decorateeFactory;
    private readonly Container container;

    // We inject a Func<T> that is able to create the command handler decoratee 
    // when needed.
    public ScopedCommandHandlerProxy(
        Func<ICommandHandler<TCommand>> decorateeFactory,
        Container container)
    {
        this.decorateeFactory = decorateeFactory;
        this.container = container;
    }

    public void Handle(TCommand command)
    {
        // Start some sort of 'scope' here that allows you to have a single 
        // instance of DbContext during that scope. How to do this depends 
        // on your DI library (if you use any).
        using (container.BeginLifetimeScope())
        {
             // Create a wrapped handler inside the scope. This way it will get 
             // a fresh DbContext.
             ICommandHandler<TCommand> decoratee =this.decorateeFactory.Invoke();

             // Pass the command on to this handler.
             decoratee.Handle(command);
        }
    }
}

This sounds a bit complex, but this completely allows you to hide the fact that a new DbContext is needed from the client Window and you hide this complexity as well from your business layer; you can simply inject a DbContext into your handler. Both sides know nothing about this little peace of infrastructure.

Of course you still have to wire this up. Without a DI library you do something like this:

var handler = new ScopedCommandHandlerProxy<MoveCustomerCommand>(
    () => new MoveCustomerCommandHandler(new DbContext()),
    container);

How to register this in a DI library is completely depending on the library of choice, but with Simple Injector you do it as follows:

// Register all command handler implementation all at once.
container.Register(
    typeof(ICommandHandler<>), 
    typeof(ICommandHandler<>).Assembly);

// Tell Simple Injector to wrap each ICommandHandler<T> implementation with a
// ScopedCommandHandlerProxy<T>. Simple Injector will take care of the rest and 
// will inject the Func<ICommandHandler<T>> for you. The proxy can be a
// singleton, since it will create the decoratee on each call to Handle.
container.RegisterDecorator(
    typeof(ICommandHandler<>), 
    typeof(ScopedCommandHandlerProxy<>),
    Lifestyle.Singleton);

This is just one of the many advantages that this type of design gives you. Other advantages is that it makes much easier to apply all sorts of cross-cutting concerns such as audit trailing, logging, security, validation, de-duplication, caching, deadlock-prevention or retry mechanisms, etc, etc. The possibilities are endless.

Answered By – Steven

Answer Checked By – Gilberto Lyons (BugsFixing Admin)