Hierarchy of classes that use a disposable object. Implement IDisposable on all of them?

Question

I have a class that uses a filestream. It needs to close the stream when the app shuts down, so I make the class implement IDisposable.

That class is a member of another class, which is a member of another class etc. All the way up to my main app.

Do I therefore have to implement IDisposable on all of these classes?

What if I change my file implementation in the future so that it closes the file after each write? I now have a whole set of classes that implement IDisposable for no reason.

I guess I'm uncomfortable with crowbarring IDisposable semantics into classes that have no need for them other than some slight implementation detail way down the chain. Are there any ways around this?

Answer 1

As a rule of thumb, when a type keeps a reference to an IDisposable in an instance field, I make it disposable too. But I usually try to avoid finding myself in this situation; when possible, I try to dispose disposables in the same method where they were created, with a using block.

Answer 2

At any given moment, every instance of every type which implements IDisposable should have at least one (and usually exactly one) entity which can be expected to call Dispose on it sometime after it is no longer needed, and before it is completely and ultimately abandoned. If your type has a field of type IDisposable, but something else can be expected to dispose of any IDisposable instance it might refer to, then you should not call Dispose on that field yourself. If your type has an IDisposable field, nobody else is going to use that object once you're done with it, and nobody else is expected to Dispose it, then you should call Dispose on the object once you no longer need it. In many cases, your object will need the other object until such time as no other object needs yours, and the way your object will find that out is when someone else calls Dispose on it (whereupon it will call Dispose on the other objects).

One pattern which can sometimes be useful is to have a class expose a Disposed event which gets raised whenever Dispose is called. This can be useful if, e.g., an another object gives your object a reference to IDisposable which it will need for awhile, and then the object that gave you the IDisposable gets done with it. It can't dispose the object while your object still needs it, and your object isn't going to dispose it (since your object won't know whether the object that supplied the IDisposable is done with it). If the class which gave your class the IDisposable hooks to your object's Disposed handler, however, the event handler can then note that your object no longer needs the IDisposable and either Dispose it immediately (if your object was the last one to need it) or set a flag so that when the other user is finished with the object it will get Disposed).

Another pattern which can be useful if your object will have a certain set of disposable objects that it will keep throughout its lifetime is to keep a List of IDisposable objects and then have your Dispose method iterate through the list and dispose things therein. Each item on the list should be Disposed within its own Try/Catch block; if an exception occurs, throw a CleanupFailureException (a custom type) which has either the first or last such exception as its InnerException, and also includes a list of all the exceptions that occurred as a custom property.

Answer 3

You need an implementation of IDisposable in each, but that doesn't necessarily require an explicit implementation in the code of each. Let inheritance do the work for you.

Two approaches:

class FileHandlingClass : IDisposable
{
  private FileStream _stm;
  /* Stuff to make class interesting */
  public void Disposable()
  {
    _stm.Dispose();
  }
  /*Note that we don't need a finaliser btw*/
}

class TextHandlingClass : FileHandlingClass
{
  /* Stuff to make class interesting */
}

Now we can do:

using(TextHandlingClass thc = new TextHandlingClass())
  thc.DoStuff();

etc.

This all works because TextHandlingClass inherits the only implementation of IDisposable it'll ever need.

It gets tricker if we have further needs for disposing though:

Say we've a class that handles pooling XmlNameTable objects (why that's a good idea is for another thread) and disposing it returns the table to the pool, and it's used by XmlHandlingClass. Now, we can deal with that to some extent with:

class XmlHandlingClass : FileHandlingClass, IDisposable
{
  PooledNameTable _table;
  /* yadda */
  public new void Dispose() // another possibility is explicit IDisposable.Dispose()
  {
    _table.Dispose();
    base.Dispose();
  }
}

Now, this works great with:

using(XmlHandlingClass x = new XmlHandlingClass())
  x.Blah();

but not with:

using(FileHandlingClass x = new XmlHandlingClass())
  x.Blah()

In the latter case only the FileHandlingClass implementation is used (luckily not returning a pooled name-table to a pool is a minor matter, most cases of Dispose() are much more crucial). Hence if needing overrides is possible, we should do:

//Allow for Disposal override
class FileHandlingClass : IDisposable
{
  private FileStream _stm;
  /* Stuff to make class interesting */
  public virtual void Disposable()
  {
    _stm.Dispose();
  }
  /*Note that we don't need a finaliser btw*/
}

//Still don't care here
class TextHandlingClass : FileHandlingClass
{
  /* Stuff to make class interesting */
}

class XmlHandlingClass : FileHandlingClass
{
  PooledNameTable _table;
  /* yadda */
  public override void Dispose()
  {
    _table.Dispose();
    base.Dispose();
  }
}

Now we have much more safety in the calls to Dispose(), but still only need to implement it ourselves where it matters.

There's a minute performance hit in the second case, but it really is minute. I'm pointing it out now only to argue against considering the first in any case where needing to override Dispose() is seen as even vaguely plausible.

Answer 4

It depends on how you implement the class that uses the filestream. If that class creates the filestream, then it should be responsible for disposing of it. However, if you were to change it so the method took in a filestream as a parameter, it would no longer 'own' the filestream and therefore not be responsible for disposing of it.

If the class is part of some kind of hierarchy, you can just add a filestream as a parameter starting at the top and introduce it to all methods down to where it is actually used.

For example:

public class Class1
{
    private readonly Class2 SomeObject = new Class2();

    public void DoWork1(Filestream stream)
    {
        SomeObject.DoWork2(stream);
    }
}

public class Class2
{
    public void DoWork2(Filestream stream)
    {
        // Do the work required with the Filestream object
    }
}

While I'm not sure I'd use this pattern myself, this will allow you to not have to add 'IDisposable' to any classes except for the one that originally created the Filestream object.

Answer 5

In general if your type contains a member that implements IDisposable the type should also implement IDiposable as well. It's the easiest way to enforce the IDisposable pattern.

The one exception I use is if my types contract contains a method which 1) must be called and 2) signals the end of use for the IDisposable resource. In that case I feel comfortable not implementing IDisposable and instead using that method to call Dispose

Answer 6

If you explicitly want to dispose the filestream, then yes, you need Implement IDisposable on any classes that hold a reference to your IDisposable. If it is reasonable to dispose the filestream after each write, i.e. does not hurt performance due to frequent wries, that sounds preferrable.