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();
    }
}

Twowheeler

I remember a similar question a few months ago. As far as I recall you can just do using(SomeObjectThatMightOrMightNotBeDisposable as IDisposable) { }. That way you don't have to explicitly check if it implements IDisposable. – Evacuate

Why not always use IDisposable? Let IFoo be an IDisposable even if Dispose wouldn't do anything needed for some implementors. – Otten

@JeroenVannevel I like that, but if the type wasn't disposable, wouldn't our instance be null when we tried to call a method on it? – Twowheeler

@Otten That's an interesting thought. That's like the null object pattern. I could have disposable methods that do nothing so all types can be treated the same way. I'll have to think about that. Thanks. – Twowheeler

@BobHorn Thats the way Streams work for example. All Streams are IDisposable even a MemoryStream who doesn't need it. – Otten

@JeroenVannevel @BobHorn you're using the as keyword but not assigning the result to anything, so nothing is set to null. I haven't seen that technique before but it looks quite clever. It'll just call Dispose if it's there and literally do nothing else. – Eyebrow

Actually, the question that I was referring to was asked by you as well. Doesn't that solve the problem? – Evacuate

Why do you want to call Dispose? Isn't it enough the resources will be freed by GC? – Lovelorn

@JeroenVannevel Ha. I wish I would have remembered that. If you want to post that as an answer, I'll accept. – Twowheeler

@BobHorn: I have added the link to Johnny's answer, you can accept it there if you deem that a good solution (with regards to Ralf's comments). – Evacuate

@Euphoric: The GC may try to notify objects when it finds they've been abandoned, but for various reason that often doesn't work well and sometimes doesn't work at all. For example, if an unbounded number of calls to GetEnumerator are made during the lifetime of a collection, and the created enumerators are subscribed to the collection's "changed" event, the collection may hold subscriptions for an unbounded number of enumerators, none of which will be eligible for cleanup during the lifetime of the collection. – Achondroplasia

@Achondroplasia And how is that related to IDisposable? – Lovelorn

@Euphoric: It's an answer to "isn't it enough that the resources will be freed by the GC"? Calling Dispose on the enumerators will clean up the subscriptions. Failing to call Dispose may prevent them from being cleaned up in timely fashion. The fact that the GC might perhaps be able to perform necessary cleanup is not enough. If an object acquires ownership of an object which implements IDisposable and might encapsulate unknown arbitrary resources, it must call Dispose. – Achondroplasia

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

Eyebrow

i downvoted this answer. Yes its clever but i think it just shows on how to not do it. The Resolve method doesn't reveal that it might be IDisposable. Should a framework user surround any returned object from any method from that framework with such a code just because it might also be IDisposable even if the returned type doesn't say so? – Otten

Fair point, I suppose this answer assumes that you're a user and you're stuck with a framework that implements it this way. – Eyebrow

@Otten why don't you add an answer where IFoo inherits IDisposable? – Eyebrow

@Otten That's a good point. How is the framework user supposed to know to even use this approach? Via some API documentation? Perhaps the best approach is to have all concrete types implement IDisposable, even if not necessary. – Twowheeler

I upvoted this answer because unless the contract being returned is inherited from IDisposable itself, the caller has no way to know it unless by documentation which isn't great way to do it. Either IFoo needs to be inherit from IDisposable or the caller needs to test for IDisposable. – Recurve

Almost any interface type IWoozle which may be returned by a factory whose callers may know nothing about the returned objects beyond the fact that they implement IWoozle should implement IDisposable if there's even a 0.00001% possibility that some returned objects may require cleanup. Implementing IDisposable in such a case doesn't give clients any responsibility they wouldn't otherwise have; it merely makes it easier for them to carry out the responsibility they do have, and makes it more likely that they'll do so. – Achondroplasia

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.

Warily

The $50,000 question should be whether there would be any plausible reason why a factory method would return an object about which nothing is known beyond the fact that it implements an interface. Such is definitely true of IEnumerator<T> (whose primary reason for being is to serve as the return type as IEnumerable<T>) but would not apply for many others. – Achondroplasia

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.

Palacios

So you're saying that IFoo should implement IDisposable, right? – Twowheeler

Yep, or rather inherit from it. Last paragraph is where I bring the analogy back to the question. – Palacios

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.

Tetrabrach