How should I inherit IDisposable?

Question

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.

Bucky

Answer 1

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.

~BaseType() => Dispose(false);

protected virtual void Dispose(bool disposing) 
{
}

void IDisposable.Dispose() 
{ 
    Dispose(true); GC.SuppressFinalize(this); 
}

Trackless

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? – Bucky

@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. – Somatic

Provided you still dispose of your managed disposable items in your Dispose method. – Somatic

@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. – Syzygy

Since asking this question, I've turned on Code Analysis and Treat warnings as errors to help enforce good disposal practices. Highly recommended. – Bucky

>"If there is a reasonable chance that an abstract entity (interface or abstract class) might need to be disposable, it should implement it. " Marc, you may be right, but this smells bad to me if I am thinking in OO correctly. If I design an interface IFooProvider, I don't want to care about all reasonable implementations of this--I just want to specify what a class must have in order to properly provide foo. Whether or not the implementing classes are disposable are concerns of the classes, right? – Whippletree

@Patrick - not quite; due to substitution principal; if the caller treats it as the base-type (without IDisposable) then they won't know to (and indeed won't be able to) dispose it correctly. Since disposal is the responsibility of the caller, it is thus necessary (in such cases) for the base implementation to advertise the need to dispose, on behalf of the possible subclasses. – Trackless

Reductio ad absurdum, shouldn't Object therefore implement IDisposable? – Whippletree

@Patrick - you can stretch it too far, obviously ;p If you were dealing with object in this was, using(obj as IDisposable) {...} is a nice trick. – Trackless

Answer 2

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.

Eelpout

Answer 3

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...

Somatic

Answer 4

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.

Napalm

Answer 5

All of the answers written so far miss a key point: it's only necessary for a base type or base interface to implement IDisposable if it's likely that code which expects an base-class instance might otherwise acquire ownership of an instance which requires disposal without realizing it. The most common scenario via which this may occur is with a factory method; a prime example is IEnumerable<T>/IEnumerator<T>. Most enumerators do not require cleanup, but code which calls IEnumerable<T>.GetEnumerator generally has no particular reason to believe that the returned enumerator will actually require cleanup, nor to believe that it won't. It's generally faster to have all implementations of IEnumerator<T> implement IDisposable, and have all consumers call Dispose on the returned enumerator, than to have consumers check whether the returned type implements IDisposable and call it if so.

If it is expected that base-type references will generally only be used by methods which will not be responsible for cleaning up the items in question, there is no need for the base type to implement IDisposable. The code which would be responsible for cleanup will know that the objects it's dealing with implement IDisposable whether or not the base type does.

Roundshouldered

Answer 6

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

Moderation