9

It was recommended to me that, when using an IOC container, I should change this:

class Foobar: IFoobar, IDisposable {};

Into this:

interface IFoobar: IDisposable{};
class Foobar : IFoobar{};

I'm wondering if this is ok, or if it solves one problem and creates another. It certainly solves the problem where I badly want to do this:

using( IFoobar = myContainer.Resolve<IFoobar>() )
{ ... }

And now I know that any substitute won't cause a run-time error.

On the other hand, now all my mock objects must handle IDisposable too. Am I right that most any mocking framework handles this easily? If yes, then perhaps this is a non-issue.

Or is it? Is there another hidden gotcha I should watch for? It certainly occurs to me that if I were using an IOC container not for unit tests / mocking, but for true service independence, then this might be a problem because perhaps only one of my swappable services actually deals with unmanaged resources (and now I'm having to implement empty "IDispose" operations in these other services).

Even this latter issue I suppose I could live with, for the sake of gaining the ability to employ the "using" statement as I demoed above. But am I following a popular convention, or am I missing an entirely different and better solution?

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
Brent Arias
  • 29,277
  • 40
  • 133
  • 234
  • 3
    Who recommended that? For what container? – Mauricio Scheffer Apr 14 '10 at 04:08
  • also have you seen http://stackoverflow.com/questions/987761/how-do-you-reconcile-idisposable-and-ioc ? – Mauricio Scheffer Apr 14 '10 at 04:12
  • The technique shown above was suggested by another engineer I work with. And yes, I had already seen that stackoverflow link. In fact, it inspired me to start this other thread: http://groups.google.com/group/structuremap-users/browse_thread/thread/6be80baf98c25820 In short the recommendation was either (1) don't use IDisposable objects with Unity or (2) don't use Unity. Round about, this gives me a total of 3 recommendations. – Brent Arias Apr 14 '10 at 22:30

1 Answers1

11

Deriving an interface from IDisposable is in my opinion a design smell that indicates a Leaky Abstraction. As Nicholas Blumhardt put it:

an interface [...] generally shouldn't be disposable. There's no way for the one defining an interface to foresee all possible implementations of it - you can always come up with a disposable implementation of practically any interface.

Consider why you want to add IDisposable to your interface. It's probably because you have a particular implementation in mind. Hence, the implementation leaks into the abstraction.

An DI Container worth its salt should know when it creates an instance of a disposable type. When you subsequently ask the container to release an object graph, it should automatically dispose the disposable components (if their time is up according to their lifestyles).

I know that at least Castle Windsor and Autofac does this.

So in your case, you should keep your type like this:

class Foobar: IFoobar, IDisposable {};

You may find Nicholas Blumhardt's post The Relationship Zoo interesting as well - particularly the discussion about Owned<T>.

Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • What about session interfaces for limited resource access (databases, hardware, etc)? Is it ok for these to be IDisposable? – xofz Apr 14 '10 at 19:53
  • IDisposable or `Owned`, but it basically boils down to the same thing. It's still a smell, but sometimes it can be necessary. One could consider the Unit of Work pattern as an alternative, but even that suffers from the same underlying problem. It's important to be aware of the smell, but sometimes there's no way to avoid it :/ – Mark Seemann Apr 14 '10 at 20:43
  • Ok, so perhaps "interface IFoobar : IDisposable" is an IOC container accomodation that with IOC containers "sometimes there's no way to avoid." But what about frequency? Suppose I incorporate an IOC container into existing code (of 300 classes) wherein it forces me to use the "smell" 6 times. Perhaps then I shrug it off. But if it affects 85 of my 300 classes, shouldn't I slam on the brakes and entirely re-evaluate what I'm doing (including whether I should ditch or switch the IOC container)? Or does the IOC container benefits usually outweight this particular smell regardless? – Brent Arias Apr 15 '10 at 19:25
  • 3
    It's rarely the DI Container itself that entails the benefits, but rather the design it implies. In any case, I would use the rule of thumb that if a change leaves the code base in a better state than before, then it's worth doing. – Mark Seemann Apr 16 '10 at 04:26
  • It's not the problem of the DI, it's the problem of the .net framework that IDisposable on implementations is required from time to time. A DI container is only responsible for Disposing if it's the owner. Waiting for the object graph to be disposed might already be late. The best way to me is: don't let the DI return a disposable object in a transient way. Use a liftime that is controlled by the DI, avoid the object to require IDisposable or provide a factory to indicate the client is the owner and should hold the object as short as possible. – Remco te Wierik Mar 05 '15 at 14:01
  • @RemcoteWierik Indeed, I describe both of the options you describe in [my book](http://amzn.to/12p90MG). – Mark Seemann Mar 05 '15 at 14:42