views:

573

answers:

5

Class names have been changed to protect the innocent.

If I have an interface named ISomeInterface. I also have classes that inherit the interface, FirstClass and SecondClass. FirstClass uses resources that must be disposed. SecondClass does not.

So the question is, where should I inherit from IDisposable? Both of the following options seem less than ideal:

1) Make FirstClass inherit IDisposable. Then, any code that deals with ISomeInterfaces will have to know whether or not to dispose of them. This smells like tight coupling to me.

2) Make ISomeInterface inherit IDisposable. Then, any class that inherits from it must implement IDisposable, even if there is nothing to dispose. The Dispose method would essentially be blank except for comments.

#2 seems like the correct choice to me, but I'm wondering if there are alternatives.

+8  A: 

If there is a reasonable chance that an abstract entity (interface or abstract class) might need to be disposable, it should implement it. Stream, for example doesn't itself need IDisposable, nor does IEnumerator<T>...

An abstract base class may be simpler, as you can have a default (empty) implementation of Dispose() then, and possibly the finalizer / Dispose(bool) pattern, i.e.

void IDisposable.Dispose() { Dispose(true); GC.SuppressFinalize(this); }
protected virtual void Dispose(bool disposing) {}
~BaseType() {Dispose(false);}
Marc Gravell
+1 for mentioning the disposal pattern.
Randolpho
Thanks, Marc! Just as a quick side question, if my class only needs to dispose of objects that are also IDisposable (rather than unmanaged resources directly), is it safe not to implement the complete pattern?
Andy West
@Andy West: very much so. The complete pattern is specifically for when you have both managed and unmanaged items to dispose. If you only have managed items to dispose, the pattern is unnecessary.
Randolpho
Provided you still dispose of your managed disposable items in your Dispose method.
Randolpho
@Andy West: It's best to implement the Basic Dispose Pattern verbatim every time to insure consistency and correctness. I have a nice ReSharper template that does it.
TrueWill
Since asking this question, I've turned on Code Analysis and Treat warnings as errors to help enforce good disposal practices. Highly recommended.
Andy West
+2  A: 

If you know some implementations of ISomeInterface require disposal, then the interface should inherit IDisposable, even if concrete implementations of the interface don't have anything to dispose of.

For instance, in the BCL, IDataReader implements IDisposable, even though one could certainly imagine data reader implementations that don't have external resources that need to be disposed of.

richardtallent
+1  A: 

My advice is go to the root and not directly to a concrete class. Point 2 is to the root and you are driven by some sort of contract by FirstClass. If you know that classes must implement some interface then you want to ensure that the interface they sign a contract iwth inherits IDisposable

JonH
+1  A: 

It depends on your interface, but I'd lean toward #2. If you have two implementations of ISomeInterface and only one needs disposing, then there's the possibility you need to refactor.

In general when you're binding to an interface, it's better to have that interface inherit IDisposable rather than the base class; if your interface doesn't inherit IDisposable, you must cast to IDisposable to dispose of the object, and that runs the risk of an InvalidCast...

Randolpho
+2  A: 

If you want all your code to deal with ISomeInterfaces generically, then yes they should all be disposable.

If not, then the code that creates FirstClass should dispose it:

using (FirstClass foo = new FirstClass()) {
    someObjectThatWantsISomeInterface.Act(foo);
}

otherwise, you could always use something like this extension method:

public static void DisposeIfPossible(this object o) {
    IDisposable disp = o as IDisposable;
    if (disp != null)
        disp.Dispose();
}

// ...
someObject.DisposeIfPossible(); // extension method on object

I should also mention that I would prefer a template base class approach to this. I stubbed this out in this blog on building disposable things properly.

plinth