Declare IDisposable for the class or interface?

Question

Starting from the following situation:

public interface ISample
{
}

public class SampleA : ISample
{
   // has some (unmanaged) resources that needs to be disposed
}

public class SampleB : ISample
{
   // has no resources that needs to be disposed
}

The class SampleA should implement the interface IDisposable for releasing resources. You could solve this in two ways:

1. Add the required interface to the class SampleA:

public class SampleA : ISample, IDisposable
{
   // has some (unmanaged) resources that needs to be disposed
}

2. Add it to the interface ISample and force derived classes to implement it:

public interface ISample : IDisposable
{
}

If you put it into the interface, you force any implementation to implement IDisposable even if they have nothing to dispose. On the other hand, it is very clear to see that the concrete implementation of an interface requires a dispose/using block and you don't need to cast as IDisposable for cleaning up. There might be some more pros/cons in both ways... why would you suggest to use one way preferred to the other?

Answer 1

Following the Inteface Segregation Principle of SOLID if you add the IDisposable to the interface you are giving methods to clients that are not interested in so you should add it to A.

Apart from that, an interface is never disposable because disposability is something related with the concrete implementation of the interface, never with the interface itself.

Any interface can be potentially implemented with or without elements that need to be disposed.

Answer 2

If you apply the using(){} pattern to all your interfaces it's best to have ISample derive from IDisposable because the rule of thumb when designing interfaces is to favor "ease-of-use" over "ease-of-implementation".

Answer 3

Personally, if all ISample's should be disposable I'd put it on the interface, if only some are I'd only put it on the classes where it should be.

Sounds like you have the latter case.

Answer 4

An interface IFoo should probably implement IDisposable if it is likely that at least some some implementations will implement IDisposable, and on at least some occasions the last surviving reference to an instance will be stored in a variable or field of type IFoo. It should almost certainly implement IDisposable if any implementations might implement IDisposable and instances will be created via factory interface (as is the case with instances of IEnumerator<T>, which in many cases are created via factory interface IEnumerable<T>).

Comparing IEnumerable<T> and IEnumerator<T> is instructive. Some types which implement IEnumerable<T> also implement IDisposable, but code which creates instances of such types will know what they are, know that they need disposal, and use them as their particular types. Such instances may be passed to other routines as type IEnumerable<T>, and those other routines would have no clue that the objects are eventually going to need disposing, but those other routines would in most cases not be the last ones to hold references to the objects. By contrast, instances of IEnumerator<T> are often created, used, and ultimately abandoned, by code which knows nothing about the underlying types of those instances beyond the fact that they're returned by IEnumerable<T>. Some implementations of IEnumerable<T>.GetEnumerator() return implementations of IEnumerator<T> will leak resources if their IDisposable.Dispose method is not called before they are abandoned, and most code which accepts parameters of type IEnumerable<T> will have no way of knowing if such types may be passed to it. Although it would be possible for IEnumerable<T> to include a property EnumeratorTypeNeedsDisposal to indicate whether the returned IEnumerator<T> would have to be disposed, or simply require that routines which call GetEnumerator() check the type of the returned object to see if it implements IDisposable, it's quicker and easier to unconditionally call a Dispose method that might not do anything, than to determine whether Dispose is necessary and call it only if so.

Answer 5

IDispoable being a very common interface, there's no harm having your interface inheriting from it. You will so avoid type checking in your code at the only cost to have a no-op implementation in some of your ISample implementations. So your 2nd choice might be better from this point of view.

Answer 6

I am starting to think that putting IDisposable on an interface can cause some problems. It implies that the lifetime of all objects implementing that interface can be safely synchronously ended. I.e., it allows anyone to write code like this and requires all implementations to support IDisposable:

using (ISample myInstance = GetISampleInstance())
{
    myInstance.DoSomething();
}

Only the code which is accessing the concrete type can know the right way to control the lifetime of the object. For example, a type might not need disposal in the first place, it might support IDisposable, or it might require awaiting some asynchronous cleanup process after you are done using it (e.g., something like option 2 here).

An interface author cannot predict all the possible future lifetime/scope management needs of implementing classes. The purpose of an interface is to allow an object to expose some API so that it can be made useful to some consumer. Some interfaces may be related to lifetime management (such as IDisposable itself), but mixing them with interfaces unrelated to lifetime management can make writing an implementation of the interface hard or impossible. If you have very few implementations of your interface and structure your code so that the interface’s consumer and lifetime/scope-manager are in the same method, then this distinction is not clear at first. But if you start passing your object around, this will be clearer.

void ConsumeSample(ISample sample)
{
    // INCORRECT CODE!
    // It is a developer mistake to write “using” in consumer code.
    // “using” should only be used by the code that is managing the lifetime.
    using (sample)
    {
        sample.DoSomething();
    }

    // CORRECT CODE
    sample.DoSomething();
}

async Task ManageObjectLifetimesAsync()
{
    SampleB sampleB = new SampleB();
    using (SampleA sampleA = new SampleA())
    {
        DoSomething(sampleA);
        DoSomething(sampleB);
        DoSomething(sampleA);
    }

    DoSomething(sampleB);

    // In the future you may have an implementation of ISample
    // which requires a completely different type of lifetime
    // management than IDisposable:
    SampleC = new SampleC();
    try
    {
        DoSomething(sampleC);
    }
    finally
    {
        sampleC.Complete();
        await sampleC.Completion;
    }
}

class SampleC : ISample
{
    public void Complete();
    public Task Completion { get; }
}

In the code sample above, I demonstrated three types of lifetime management scenarios, adding to the two you provided.

1. SampleA is IDisposable with synchronous using () {} support.
2. SampleB uses pure garbage collection (it does not consume any resources).
3. SampleC uses resources which prevent it from being synchronously disposed and requires an await at the end of its lifetime (so that it may notify the lifetime management code that it is done consuming resources and bubble up any asynchronously encountered exceptions).

By keeping lifetime management separate from your other interfaces, you can prevent developer mistakes (e.g., accidental calls to Dispose()) and more cleanly support future unanticipated lifetime/scope management patterns.

Answer 7

Personally I would choose 1, unless you make a concrete example for two. A good example of two is an IList.

An IList means you need to implement an indexer for your collection. However, an IList really also means you are an IEnumerable, and you should have a GetEnumerator() for your class.

In your case you are hesistant that classes that implement ISample would need to implement IDisposable, if not every class that implements your interface has to implement IDisposable then don't force them to.

Focusing on IDispoable specifically, IDispoable in particular forces programmers using your class to write some reasonably ugly code. For example,

foreach(item in IEnumerable<ISample> items)
{
    try
    {
        // Do stuff with item
    }
    finally
    {
        IDisposable amIDisposable = item as IDisposable;
        if(amIDisposable != null)
            amIDisposable.Dispose();  
    }
}

Not only is the code horrible, there will be a significant performance penalty in ensuring there is a finally block to dispose of the item for every iteration of that list, even if Dispose() just returns in the implementation.

Pasted the code to answer one of the comments in here, easier to read.