Factory pattern and lifetime of injected dependencies dilemma

Question

This has been bothering me for a long time, and I could not find the right answer.

The Problem.

Imagine you have a factory interface (C# example):

interface IFooFactory
{
    IFoo Create();
}

and it's implementation depends on a service:

class FooFactory : IFooFactory
{
    private readonly IBarService _barService;
    public FooFactory(IBarService barService)
    {
        _barService = barService;
    }
}

where the service interface implements IDisposable for a graceful shutdown:

interface IBarService : IDisposable
{
    ...
}

Now the actual class created by the factory has 2 dependencies - the service itself (passed through the factory), and another object that is created by factory:

class Foo : IFoo
{
    public Foo(IBarService barService, IQux qux)
    {
        ...
    }
}

and the factory can create it like this:

class FooFactory : IFooFactory
{
    public IFoo Create()
    {
        IQux qux = new Qux();
        return new Foo(_barService, qux);
    }
}

Finally IFoo and IQux both implement IDisposable as well, so the class Foo does:

class Foo : IFoo
{
    public void Dispose()
    {
        _qux.Dispose();
    }
}

But why do we dispose Qux only on Foo.Dispose()? Both dependencies are injected and we just rely on the knowledge of exact implementation of the factory, where Bar is a shared service (association relationship type) and the Qux is used by Foo exclusively (composition relationship type). It can be easy to dispose both of them by mistake. And in both cases Foo logically does not own any of the dependencies, so disposing any of them seems wrong. Putting creation of Qux inside Foo would negate the dependency injection, so it's not an option.

Is there a better way to have both dependencies and make it clear what kind of the relationship they have to properly handle their lifetime?

Possible solution.

So here is one possible not that pretty solution:

class FooFactory : IFooFactory
{
    private readonly IBarService _barService;
    public FooFactory(IBarService barService)
    {
        _barService = barService;
    }
    public IFoo Create()
    {
        // This lambda can capture and use any input argument.
        // Also creation can be complex and involve IO.
        var quxFactory = () => new Qux();
        return new Foo(_barService, quxFactory);
    }
}
class Foo : IFoo
{
    public Foo(IBarService barService, Func<IQux> quxFactory)
    {
        // Injected - don't own.
        _barService = barService;
        // Foo creates - Foo owns.
        _qux = quxFactory();
    }
    public void Dispose()
    {
        // Now it's clear what Foo owns from the code in the constructor.
        _qux.Dispose();
    }
}

I'm not a fan of calling possibly complex logic in a constructor, especially if it's async, and calling it on-demand (lazy load) can also lead to unexpected late runtime errors (vs. failing fast).

Does it really make sense to go that far just for the sake of the design? In any case I'd like to see if there any other possible elegant solution to this.

Answer 1

First of all this:

interface IBarService : IDisposable
{
    ...
}

is a leaky abstraction. We only know that BarService has a constructor-injected disposable dependency. That is not a guarantee that every implementation of IBarService will need to be disposable. To remove the leaky abstraction, IDisposable should only be applied to the concrete implementations that actually require it.

interface IBarService
{
    ...
}

class BarService : IBarService, IDisposable
{
    ...
}

There is a register, resolve, release pattern at play when dealing with dependeny injection. Also, per MSDN guidelines the party that creates a disposable is also responsible for disposing it.

In general that means when using a DI container, the DI container is responsible for both instantiating and disposing the instance.

However, the DI pattern also allows for the possibility of wiring up components without a DI container. So, to be 100% thorough we should design components with disposable dependencies in mind.

In short that means:

1. We need to make Foo disposable, since it has a disposable dependency
2. Since the caller of the factory is responsible for instantiating Foo, we need to provide the caller of the factory with a way to dispose Foo

As pointed out in this article, the most elegant way is to provide a Release() method that allows the caller to inform the factory it is finished with the instance. It is then up to the factory implementation to decide whether to dispose the instance explicitly, or to delegate to a higher power (the DI container).

interface IFooFactory
{
     IFoo Create();
     void Release(IFoo foo);
}

class FooFactory : IFooFactory
{
    private readonly IBarService _barService;
    private readonly IQux qux;
    public FooFactory(IBarService barService, IQux qux)
    {
        _barService = barService;
        _qux = qux;
    }
    public IFoo Create()
    {
        return new Foo(_barService, _qux);
    }
    public void Release(IFoo foo)
    {
        // Handle both disposable and non-disposable IFoo implementations
        var disposable = foo as IDisposable;
        if (disposable != null)
            disposable.Dispose();
    }
}

class Foo : IFoo, IDisposable
{
    public Foo(IBarService barService, IQux quxFactory)
    {
        _barService = barService;
        _qux = quxFactory;
    }

    public void Dispose()
    {
        _barService.Dispose();
        _qux.Dispose();
    }
}

And then the factory usage pattern looks something like:

// Caller creates the instance, the caller owns
var foo = factory.Create();
try
{
    // do something with foo
}
finally
{
    factory.Release(foo);
}

The Release method guarantees 100% that the disposables will be properly disposed whether the consuming application is wired up using a DI container or using pure DI.

Note that it is the factory that decides whether or not to dispose IFoo. So when using a DI container, the implementation could be omitted. However, since it should be safe to call Dispose() more than once we could just as well leave it in place (and possibly use a boolean to ensure dispose is not called more than once on buggy components that don't allow for the possibility).

Answer 2

You are newing up Qux everytime, if you can hand that off to the DI framework then you wouldn't have to call dispose, the framework work will when it has to