DI and Lifetime Management

Question

what is the 'best' way to manage the lifecycle of a disposable object when it is injected into another class. The example I keep running into is when running database queries using entity framework in a class that has a long lifetime.

Here is an example

public class CustomerViewModel
{
  private ICustomerRepository _customerRepository;

  public CustomerViewModel(ICustomerRepository customerRepository)
  {
    _customerRepository = customerRepository;
  }

  public void AddCustomer(Customer customer)
  {
     _customerRepository.Customers.Add(customer);
     _customerRepository.SaveChanges();
  }
}

The code above looks perfectly innocent to me, however the _customerRepository object exists for as long as the CutomerViewModel exists for, which is much longer than it should.

If I were writting this code without DI i would do this:

public class CustomerViewModel
{
  public void AddCustomer(Customer customer)
  {
     using (var customerRepository = new CustomerRepository())
     {
       customerRepository.Customers.Add(customer);
       customerRepository.SaveChanges();
     }
  }
}

Which handles the lifecycle of CustomerRepository correctly.

How is this supposed to be managed when a class requires a Disposable object to have a shorter lifetime than itself?

The method I am using now is to create a RepositoryCreator object, which knows how to initialize a repository, but this feels wrong:

public class CustomerViewModel
{
  private ICustomerRepositoryCreator _customerRepositoryCreator;

  public CustomerViewModel(ICustomerRepositoryCreator customerRepositoryCreator)
  {
    _customerRepositoryCreator= customerRepositoryCreator;
  }

  public void AddCustomer(Customer customer)
  {
     using (var customerRepository = _customerRepositoryCreator.Create())
     {
       customerRepository.Customers.Add(customer);
       customerRepository.SaveChanges();
     }
  }
}

UPDATE

So would doing something like this be preferable, it could be made generic but for the sake of this example I will not do this.

public class CustomerViewModel
{
  private ICustomerRepository _customerRepository;

  public CustomerViewModel(ICustomerRepository customerRepository)
  {
    _customerRepository = customerRepository;
  }

  public void AddCustomer(Customer customer)
  {
     _customerRepository.Add(customer);
  }
}


public class CustomerRepository : ICustomerRepository 
{
   private readonly DbContext _dbContext;

   public CustomerRepository(DbContext dbContext)
   {
      _dbContext = dbContext;
   }

   public void Add(Customer customer)
   {
      _dbContext.Customers.Add(customer);
      _dbContext.Customers.SaveChanges();
   }
}

And have a proxy which manages lifetime

public class CustomerRepositoryLifetimeProxy : ICustomerRepository 
{
    private readonly _container;
    public CustomerRepositoryLifetimeProxy(Container container)
    {
       _container = container;
    }

    public void Add(Customer customer)
    {
      using (Container.BeginScope()) //extension method
      {
          ICustomerRepository cRepo = Container.Resolve<ICustomerRepository>();
          cRepo.Add(customer);
       } // releases the instance
    }
}

If this is better, should the Proxy know about the DI container, or should it rely on a factory?

Having a 'factory' of sorts is not really bad, especially since you are in full control how the repository is disposed. If the creation logic is simple, you can inject `Func directly instead of creating a separate factory type. — Patryk Ćwiek, May 21 '14 at 10:45
@PatrykĆwiek, should have mentioned that most times I use the interface `IRepositoryCreator` for the creation object. I never liked the `Creator` suffix don't know why I didn't use `Factory`!!! — Nick Williams, May 21 '14 at 10:53
The reason it *should* feel wrong is because adding a factory is a **Leaky Abstraction**. The only reason you do that is because you know that the *implementation* is disposable. A better approach is described in this other SO answer: http://stackoverflow.com/a/4650050/126014 — Mark Seemann, May 21 '14 at 10:59
@MarkSeemann how does the wrapper know **when** to dispose of the object? — Nick Williams, May 21 '14 at 11:08
@MarkSeemann but in some cases we do know that the implementation is disposable. For example it's okay for me to implement `Unit Of Work` pattern adding `IDisposable` to it's interface and otherwise it would be too difficult to work with. — Stepan Burguchev, May 21 '14 at 11:11
@MarkSeemann: I don't agree. If the repository contracts (i.e. the interface) inherits `IDisposable` it's established that the repositories should be cleaned up. Most data sources do required cleanup when used. — jgauffin, May 21 '14 at 11:12
@jgauffin: But if the consumer of the repository doesn't need to dispose that repository itself, you will be violating the [ISP](https://en.wikipedia.org/wiki/Interface_segregation_principle) when you implement IDisposable, since now the client depends on a method that it doesn't use (and probably shouldn't use). — Steven, May 21 '14 at 11:29
@NickWilliams If you want to adhere strictly to the Liskov Substitution Principle, you'll have to design the interface in such a way that you can complete the operation in a single method call. You can read more in chapter 6 of [my book](http://amzn.to/12p90MG). — Mark Seemann, May 21 '14 at 11:32
@sburg As originally(?) described in [PoEAA](http://amzn.to/13QdgJ6), the Unit of Work pattern doesn't implement IDisposable (because that's a .NET-specific interface), but rather defines a Commit method. There's a subtle difference there. Really, using IDisposable for Unit of Work is to repurpose IDisposable for something it wasn't originally intended. One can argue for and against that... — Mark Seemann, May 21 '14 at 11:36
@jgauffin Indeed, *if* the interface derives from IDisposable I agree, but as I also explain in [my book](http://amzn.to/12p90MG), any *interface* that derives from IDisposable is a Leaky Abstraction, so should be redesigned. The original argument isn't mine, but Nicholas Blumhardt's. — Mark Seemann, May 21 '14 at 11:40

Steven · Accepted Answer · 2014-08-11T12:02:26.697

The problem here is that your AddCustomer method in your ViewModel does to much. The viewmodel should not be responsible of handling business logic, and the repositories consumer shouldn't know nothing about committing a unit of work and should not be able to add a customer to the list of customers.

So instead, refactor your ICustomerResository to the following:

public interface ICustomerRepository
{
    void Add(Customer customer);
}

In this case, the Add method should be atomic and do the commit itself. This way the viewmodel can depend on that interface and in case the viewmodel outlives the customer repository, you can wrap the real repository with a proxy:

public class CustomerRepositoryProxy : ICustomerRepository
{
    private readonly Func<ICustomerRepository> repositoryFactory;

    public CustomerRepositoryProxy(Func<ICustomerRepository> repositoryFactory) {
        this.repositoryFactory = repositoryFactory;
    }

    public void Add(Customer customer) {
        var repository = this.repositoryFactory.Invoke();
        repository.Add(customer);
    }
}

Of course this will start to become quite cumbersome if you have dozens of those IXXXRepository interfaces. In that case, you might want to migrate to one generic interface instead:

public interface IRepository<TEntity>
{
    void Add(TEntity entity);
}

This way you can have one single proxy for all repositories:

public class RepositoryProxy<TEntity> : IRepository<TEntity>
{
    private readonly Func<IRepository<TEntity>> repositoryFactory;

    public CustomerRepositoryProxy(Func<IRepository<TEntity>> repositoryFactory) {
        this.repositoryFactory = repositoryFactory;
    }

    public void Add(TEntity entity) {
        var repository = this.repositoryFactory.Invoke();
        repository.Add(entity);
    }
}

In that case (assuming you wire your object graphs by hand) you can build up the viewmodel as follows:

new CustomerViewModel(
    new RepositoryProxy<Customer>(
        () => new CustomerRepository(unitOfWork)));

You can even take it one step further and implement the command/handler pattern and query/handler pattern. In that case you don't inject a IRepository<Customer> into your view model, but you inject an ICommandHandler<AddCustomer> into the view model and instead of injecting the AddCustomerCommandHandler implementation into the view model, you inject a proxy that creates the real handler when needed:

public class LifetimeScopedCommandHandlerProxy<TCommand> : ICommandHandler<TCommand>
{
    private readonly Func<ICommandHandler<TCommand>> decorateeFactory;

    public LifetimeScopedCommandHandlerProxy(
        Func<ICommandHandler<TCommand>> decorateeFactory) {
        this.decorateeFactory = decorateeFactory;
    }

    public void Handle(TCommand command) {
        var decoratee = this.decorateeFactory.Invoke();
        decoratee.Handle(command);
    }
}

The view model will look as follows:

public class CustomerViewModel
{
    private ICommandHandler<AddCustomer> addCustomerCommandHandler;

    public CustomerViewModel(ICommandHandler<AddCustomer> addCustomerCommandHandler) {
        this.addCustomerCommandHandler = addCustomerCommandHandler;
    }

    public void AddCustomer(Customer customer)
    {
        this.addCustomerCommandHandler.Handle(new AddCustomer(customer));
    }
}

And the object graph will look similar as before:

new CustomerViewModel(
    new LifetimeScopedCommandHandlerProxy<AddCustomer>(
        () => new AddCustomerCommandHandler(unitOfWork)));

Of course, it will be much easier building these object graphs when using a container.

UPDATE

If you use a DI container, and you're not running in something like a web request, you will have to start a new 'scope' or 'request' explictly to inform the container what to do. With Simple Injector your proxy will looks like this:

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

    // Here we inject the container as well.
    public LifetimeScopedCommandHandlerProxy(Container container, 
        Func<ICommandHandler<TCommand>> decorateeFactory) 
    {
        this.container = container;
        this.decorateeFactory = decorateeFactory;
    }

    public void Handle(TCommand command) {
        // Here we begin a new 'lifetime scope' before calling invoke.
        using (container.BeginLifetimeScope())
        {
            var decoratee = this.decorateeFactory.Invoke();
            decoratee.Handle(command);
        }
        // When the lifetime scope is disposed (which is what the using
        // statement does) the container will make sure that any scope
        // instances are disposed.
    }
}

In that case your configuration might look like this:

// This instance lives as long as its scope and will be disposed by the container.
container.RegisterLifetimeScope<IUnitOfWork, DisposableUnitOfWork>();

// Register the command handler
container.Register<ICommandHandler<AddCustomer>, AddCustomerCommandHandler>();

// register the proxy that adds the scoping behavior
container.RegisterSingleDecorator(
    typeof(ICommandHandler<>),
    typeof(LifetimeScopedCommandHandlerProxy<>));

container.Register<CustomerViewModel>();

I agree the viewmodel in this example does too much. Does the example you give at the top do the same as my last example, but in a more appropriate place? — Nick Williams, May 21 '14 at 12:11
@NickWilliams: The big difference is, that I didn't introduce a new interface for the client. The client will be oblivious to this implementation detail. And note that my `CustomerRepositoryProxy` does not dispose the repository, but leaves that up to the part of the system that is responsible for creating that instance (usually your DI container). — Steven, May 21 '14 at 12:14
I agree that the client shouldn't know about a new Factory type interface. So even though you don't dispose of the repository in the add method, you still create a new repository object using the `this.repositoryFactory.Invoke();` method? Given that the repository does implement `IDisposable` couldn't this cause a memory leak, as the GC doesn't know whether to destroy it? I thought that was why the `using` block should be used. — Nick Williams, May 21 '14 at 12:22
@Nick: It depends. In the way I build the object graph in the example, a new repository is created on each invoke. In reality, the call to `Invoke` will callback into the DI framework to get the proper instance, and in that case the framework will ensure the correct scoping of the item and the correct disposing of that item. But even if you don't have a DI framework, you can implement this manually (but will take more work). — Steven, May 21 '14 at 12:24
I see, I think for my use at the moment I will have to dispose of the repository manually, I am unsure as to how the DI framework can know whether or not it should be dissposed / recreated. — Nick Williams, May 21 '14 at 12:27
@NickWilliams: Well, that's what DI containers are for. Composing object graph and managing the lifetime of objects. — Steven, May 21 '14 at 12:56
@NickWilliams: See my update. I added an example of how to do this with a DI container. You can achieve the same manually, but have to create some sort of 'Scope' object yourself. — Steven, May 21 '14 at 13:02
I have only used DI containers for composing an object graph, I didn't realise that they could be used for lifetime management. At the moment I am using castlewindsor and I do use the lifetime management such that objects are transient or singletons. Seemingly yo use scoped life times I would have to inject the container into required properties.Which seems worse than a Factory mathod. See [here](http://docs.castleproject.org/Default.aspx?Page=LifeStyles&NS=Windsor&AspxAutoDetectCookieSupport=1#Scoped_7) — Nick Williams, May 23 '14 at 12:02
@NickWilliams: There must be some misunderstanding. There should be no need to inject the container anywhere. In my example, the only class that depends on the container is the `LifetimeScopedCommandHandlerProxy`, but that's [okay](http://blog.ploeh.dk/2011/08/25/ServiceLocatorrolesvs.mechanics/), since it is part of your [Composition Root](http://blog.ploeh.dk/2011/07/28/CompositionRoot/). — Steven, May 23 '14 at 12:06
I think I am misunderstanding something, probably because I don't know what the `LifetimeScopedCommandHandlerProxy` does. I have edited my answer with a non-generic guess at what it's doing. Am I at all close? — Nick Williams, May 23 '14 at 12:53
@NickWilliams: You're update is exactly what I'm suggesting and yes: the proxy can know about the container, that's not a problem (as long as this proxy is defined near your DI configuration). I'm sorry if my answer is making steps that are too big. However, note the use of those generic types will get very valuable very soon, since you'd be creating many off-by-one proxies and will start to get lots of duplicate code. — Steven, May 23 '14 at 13:05
Completely agree with you. Just for my understanding it's easier for me to use specifics, it's fairly trivial to make items generic though. The only downside to this method is that on some method like `GetCustomers` you can't return an `IQueryable` as the dbContext has been dissposed. Having said that `IQueryable` is a leaky abstraction anyway, I suppose that's what the specification pattern is for. — Nick Williams, May 23 '14 at 13:09
Your steps weren't too big anyway, I was just being dense. Passing the DI container around is something I am always wary about as I have committed a few Service Locator sins in my past. You're quite right though, so long as it exists as near to the entry point as possible. — Nick Williams, May 23 '14 at 13:11
@NickWilliams: Leaky abstraction it is. I always prevent returning an IQueryable implementation from my business layer. What I do is passing all required information to do the filtering and sorting in the BL. I use [this pattern](https://bit.ly/1dpeBdl) for specifying queries in my BL. — Steven, May 23 '14 at 13:14
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/54280/discussion-between-steven-and-nick-williams). — Steven, May 23 '14 at 13:20

score 0 · Answer 2 · answered May 21 '14 at 10:51

In general it is up to the creator to dispose a disposable object as soon es it is done with its usage. If your injected object can live through entire application lifecycle, i.e. without needing to dispose it in the meantime, than the normal DI approach (your first code block) is a good way to go. However, if you need to dispose the object as soon as possible, than a factory approach makes much more sense (last code block).

DI and Lifetime Management

2 Answers2