Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Polymorphism when concrete types *might* be disposable

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();
    }
}
like image 730
Bob Horn Avatar asked Jan 06 '14 15:01

Bob Horn


3 Answers

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

like image 140
johnnycardy Avatar answered Oct 25 '22 13:10

johnnycardy


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.

like image 29
Jon Hanna Avatar answered Oct 25 '22 13:10

Jon Hanna


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.

like image 2
Mark Bostleman Avatar answered Oct 25 '22 15:10

Mark Bostleman