16

I have a class hierarchy, each member of which may create IDisposable objects.

I added a List<IDisposable> property to the base class in this hierarchy, to which I add any disposable objects on creation. The root Dispose method iterates through this list and calls Dispose for each item in its list and clears the list. In the application, I explicitly call the top object's Dispose method, causing disposal to cascade through the hierarchy.

This works, but is there a better way? Am I unwittingly duplicating some functionality already present in the framework?

(Note - the objects in question have a lifetime that precludes just wrapping them in a using block or disposing of them in the same methodwhere they are created.)

Edit

Just for clarification - I'm only keeping those objects around that need to be kept. Some are disposed of in the same method where they are created, but many are used in such a way that this isn't possible.

3Dave
  • 28,657
  • 18
  • 88
  • 151

5 Answers5

14

No that is correct. IDisposable is designed to free up unmanaged resources and should be called as soon as possible after you have finished with the instance. It's a common misconception that this is unnecessary, or that the finailizer will do this automatically when the object is garbage collected. It does not.

The correct pattern for IDisposable is here, and included below for quick reference.

public class Resource : IDisposable 
{
    // Dispose() calls Dispose(true)
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    // NOTE: Leave out the finalizer altogether if this class doesn't 
    // own unmanaged resources itself, but leave the other methods
    // exactly as they are. 
    ~Resource() 
    {
        // Finalizer calls Dispose(false)
        Dispose(false);
    }

    // The bulk of the clean-up code is implemented in Dispose(bool)
    protected virtual void Dispose(bool disposing)
    {
        if (disposing) 
        {
            // free managed resources
        }
        // free native resources here if there are any
    }
}
TheCodeKing
  • 19,064
  • 3
  • 47
  • 70
12

So long as you implement the disposable pattern correctly (as described here), this method is fine.

As far as I know, only using statements have special support for IDisposable - there isn't anything in the framework that replicates what you are doing.

Oded
  • 489,969
  • 99
  • 883
  • 1,009
2

If you're talking about arbitrary IDisposable objects, I don't believe it exists.

The System.ComponentModel.Container class implements cascading Dispose, but requires the elements to implement IComponent. If you control your IDisposable objects you could make them implement IComponent - it only requires implementing a single property Site that can return null.

Joe
  • 122,218
  • 32
  • 205
  • 338
  • I'm not sure this is a good idea - the `IComponent` interface is intended for non-visual components intended to be used in a RAD designer, which seems unlikely in a general class hierarchy. – Greg Beech Sep 04 '11 at 16:28
  • @Greg, I'm pointing out its existence rather than recommending it for a given scenario. But if you don't think it's a good idea, maybe you could explain why? – Joe Sep 04 '11 at 16:31
  • 1
    @Joe Good info. I should have mentioned that some of this runs on the compact framework, where IComponent doesn't appear to be supported. – 3Dave Sep 04 '11 at 16:34
  • @Joe - As I said, implementing `IComponent` indicates that the object can be used in a RAD designer (i.e. dropped as a component on a windows form) and should have the associated design-time support for that scenario. That is probably not the case for most class hierarchies. Have a read of http://msdn.microsoft.com/en-us/library/0b1dk63b.aspx and its linked pages for more information. – Greg Beech Sep 04 '11 at 16:38
  • @Greg - useful to have this info as it helps to assess whether the recommendation applies to a given scenario. In this case, I'm still not convinced - thare are IComponent classes in the Framework that are not mainly intended for use in a RAD designer - e.g. System.Diagnostics.Process. – Joe Sep 09 '11 at 12:58
1

Sounds like a situation where the visitor pattern could be appropriate. Although I never understand the claim that it extends your classes and leaves them unchanged, because I only know examples where classes should have an AcceptVisitor method or the like. BTW, it's not a pattern I like because it is complex and tends to clutter code.

Gert Arnold
  • 105,341
  • 31
  • 202
  • 291
1

If one object will create many other IDisposable objects and maintain ownership of them throughout its existence, the pattern you describe can be a good one. It may be enhanced by having your class implement method "T RegDispos<T>(T thing) where T:IDisposable;" which will add a disposable to the list and return it. One can thus take care of the creation and cleanup of a resource in the same statement, by replacing a statement like "someField = someDisposType.CreateThing();" with "someField = RegDispos(someDisposType.CreateThing());".

If your class doesn't expose a public constructor (requires outsiders use factory methods), and if you're either using vb.net or are willing to use thread-static fields, you can even combine initialization and cleanup with declaration (e.g. "var someField = RegDispos(someDisposType.CreateThing());"). For this to be safe, the constructor must be invoked within a try/catch or try/finally block that can call Dispose on created sub-objects if construction fails. Because field initializers in C# don't have access to constructor parameters (a weakness in the language, IMHO) the only way to implement such a pattern is to have a factory method create the list and put it into a thread-static variable which can then be read by a static RegDispos method.

supercat
  • 77,689
  • 9
  • 166
  • 211