Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Passing IDisposable objects through constructor chains

I've got a small hierarchy of objects that in general gets constructed from data in a Stream, but for some particular subclasses, can be synthesized from a simpler argument list. In chaining the constructors from the subclasses, I'm running into an issue with ensuring the disposal of the synthesized stream that the base class constructor needs. Its not escaped me that the use of IDisposable objects this way is possibly just dirty pool (plz advise?) for reasons I've not considered, but, this issue aside, it seems fairly straightforward (and good encapsulation).

Codes:

abstract class Node {
    protected Node (Stream raw)
    {
        // calculate/generate some base class properties
    }
}
class FilesystemNode : Node {
    public FilesystemNode (FileStream fs)
        : base (fs)
    {
        // all good here; disposing of fs not our responsibility
    }
}
class CompositeNode : Node {
    public CompositeNode (IEnumerable some_stuff)
        : base (GenerateRaw (some_stuff))
    {
        // rogue stream from GenerateRaw now loose in the wild!
    }

    static Stream GenerateRaw (IEnumerable some_stuff)
    {
        var content = new MemoryStream ();
        // molest elements of some_stuff into proper format, write to stream
        content.Seek (0, SeekOrigin.Begin);
        return content;
    }
}

I realize that not disposing of a MemoryStream is not exactly a world-stopping case of bad CLR citizenship, but it still gives me the heebie-jeebies (not to mention that I may not always be using a MemoryStream for other subtypes). It's not in scope, so I can't explicitly Dispose () it later in the constructor, and adding a using statement in GenerateRaw () is self-defeating since I need the stream returned.

Is there a better way to do this?

Preemptive strikes:

  • yes, the properties calculated in the Node constructor should be part of the base class, and should not be calculated by (or accessible in) the subclasses
  • I won't require that a stream be passed into CompositeNode (its format should be irrelevant to the caller)
  • The previous iteration had the value calculation in the base class as a separate protected method, which I then just called at the end of each subtype constructor, moved the body of GenerateRaw () into a using statement in the body of the CompositeNode constructor. But the repetition of requiring that call for each constructor and not being able to guarantee that it be run for every subtype ever (a Node is not a Node, semantically, without these properties initialized) gave me heebie-jeebies far worse than the (potential) resource leak here does.
like image 440
Matt Enright Avatar asked Jun 07 '10 11:06

Matt Enright


2 Answers

CompositeNode created the stream - CompositeNode has the responsibility unless some other code explicitly states it'll take that on. One option here would be for the base-constructor to allow this via a protected overload, but the ordering means that it would be hard to know exactly when it is safe to dispose it. A virtual method would allow you to do this, but you shouldn't really call virtual methods in constructors.

I wonder if refactoring so that there is an initialize (Load) method (that you call separately to construction) would be better. Perhaps a protected virtual method, and expose it via a public static method.

like image 128
Marc Gravell Avatar answered Oct 12 '22 11:10

Marc Gravell


You may want to consider passing instructions regarding disposition as/within a separate argument of the constructor that accepts an IDisposable. This is the approach taken by XmlReader.Create, which accepts an XmlReaderSettings parameter whose CloseInput property determines whether the underlying data source is disposed when the created XmlReader is eventually disposed.

like image 24
Nicole Calinoiu Avatar answered Oct 12 '22 12:10

Nicole Calinoiu