Polymorphism when concrete types *might* be disposable

Question

If a dependency container, or data access factory, can return types that may implement IDisposable, should it be the client's responsibility to check for that and handle it? In my code below, one data class implements IDisposable and the other doesn't. Either can be returned by the data access factory.

    private static void TestPolymorphismWithDisposable()
    {
        // Here we don't know if we're getting a type that implements IDisposable, so
        // if we just do this, then we won't be disposing of our instance.
        DataAccessFactory.Resolve().Invoke();

        // Do we just have to do something like this?
        IFoo foo = DataAccessFactory.Resolve();
        foo.Invoke();
        var fooAsDisposable = foo as IDisposable;
        if (fooAsDisposable != null) { fooAsDisposable.Dispose(); }
    }

The reason I ask is that this seems to place a burden on the client code to have to handle this, and how do you let clients know, when building an API, that they may need to call dispose? Is there a better way to handle this where the client doesn't have to check for IDisposable?

In the interest of a full working example, here are the other classes:

public interface IFoo
{
    void Invoke();
}

public class DataAccessBaseNotDisposable : IFoo
{
    public void Invoke()
    {
        Console.WriteLine("In Invoke() in DataAccessBaseNotDisposable.");
    }
}

public class DataAccessBaseDisposable : IFoo, IDisposable
{
    public void Invoke()
    {
        Console.WriteLine("Invoke() in DataAccessBaseDisposable.");
    }

    public void Dispose()
    {
        Console.WriteLine("In Dispose() in DataAccessBaseDisposable.");
    }
}

public static class DataAccessFactory
{
    public static IFoo Resolve()
    {
        return new DataAccessBaseDisposable();
        //return new DataAccessBaseNotDisposable();
    }
}

Answer 1

Edit: The best solution is to always return an IDisposable object, even if the object doesn't need to be disposed. That way, the framework user doesn't have to keep checking for IDisposable all the time.

Your framework would look like this:

public interface IFoo : IDisposable
{
    void Invoke();
}

public class DataAccessBase : IFoo
{
    public void Invoke()
    {
        Console.WriteLine("In Invoke() in DataAccessBase.");
    }

    public void Dispose()
    {
        Console.WriteLine("In Dispose() in DataAccessBase.");
    }
}

public static class DataAccessFactory
{
    public static IFoo Resolve()
    {
        return new DataAccessBase();
    }
}

And it is consumed as you'd expect:

private static void TestPolymorphismWithDisposable()
{
    using(IFoo foo = DataAccessFactory.Resolve())
    {
        foo.Invoke();
    }
}

However, If you're a user and you're stuck with a result which may or may not implement IDisposable, you would need to consume it as follows:

private static void TestPolymorphismWithDisposable()
{
    IFoo foo = DataAccessFactory.Resolve();

    using(foo as IDisposable)
    {
        foo.Invoke(); //This is always executed regardless of whether IDisposable is implemented

        //Dispose is called if IDisposable was implemented
    }
}

See these questions: Using statement with a null object and Using 'as IDisposable' in a using block

Answer 2

I think having IFoo inherit IDisposable is the way to go. The only exception might be if there is a fixed number of IFoo implementations, the framework author owns them (that is, they cannot be or are not intended to be provided by others), and none of them are disposable.

Answer 3

Compare with how IEnumerator (first defined in .NET 1.0) differs from IEnumerator<T> (first defined in .NET 2.0).

With the former, a foreach like:

foreach(string s in SomeStringSource)
{
  Console.WriteLine(s);
}

is treated as:

var en = SomeStringSource.GetEnumerator();
try
{
  while(en.MoveNext())
  {
    string s = (string)en.Current;
    Console.WriteLine(s);
   }
}
finally
{
  if(en is IDisposable)
    ((IDisposable)en).Dispose();
}

(Of course not really var since it's .NET 1.0, but there is a similar compile-time type-inference involved).

With the latter, because IEnumerable<T> is defined as inheriting from IDisposable, the same code is treated as:

using(var en = SomeStringSource.GetEnumerator())
  while(en.MoveNext())
  {
    string s = (string)en.Current;
    Console.WriteLine(s);
   }

Now, foreach hides this from us much of the time, but:

1. The second approach is much nicer to deal with in those cases you do need to handle an IEnumerator<T> yourself.
2. The second approach is nicer to implement; you are reminded that disposal is often an issue with enumerators, so you are unlikely to forget it (I saw more than one old pre-generics enumerator that failed to dispose when it should have).

So, by analogy, we can take the view that having IFoo inherit from IDisposable, and hence always having a Dispose() implementation (albeit possibly empty), is supported by the experience that led to the .NET team making that change from 1.0->2.0, along with being an approach many users are used to (IEnumerator<T> is one of the most commonly used types after all). Further, failure to dispose can be caught by code analysis tools like FxCop.

Answer 4

Questions like these should be answered basing not on convenience of the user, rather basing on the meaning of the concepts involved.

In your case you're asking about a scenario which involves data access. Conceptually, it's reasonable to expect that a data access class will need to dispose of unmanaged resources, like database connections, file handles, etc. So the hypothetical IFoo probably should inherit from IDisposable because it makes sense that it does due to its nature, and not because it's convenient for the user.

If we expand this topic to a general purpose dependency injection or factory system, one which creates objects which do various tasks, it does not make sense to have all of the interfaces inherit from IDisposable just because some section of them can have it. This is a huge anti-pattern.

The way dependency injection frameworks accomplish this is by maintaining a "scope" which tracks all of the disposable services it has provided, so that when the scope is disposed, it can then iterate over the disposables collection and dispose them, too. Now that would be the right approach.

This brings us back to your situation. It's unclear how you are deciding which implementation of IFoo to return, but it may be useful to abandon this approach and go use a dependency injection framework right away, before you go the wrong direction and dig yourself a deep hole trying to reinvent one.