views:

71

answers:

1

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?

+4  A: 

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
Nice Answer. Sounds all good.
Jehof
What about session interfaces for limited resource access (databases, hardware, etc)? Is it ok for these to be IDisposable?
Sam Pearson
IDisposable or `Owned<T>`, 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
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
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