Search code examples
c#usingidisposable

Do i need to Dispose Stream when i Pass it to IDisposable class?


I wrote a piece of code. I want to make sure that I am Disposing an Object in right way.

I have a Disposable Class like this Which is used to read some data from unmanaged resource.

class MyFileReader : IDisposable
{
    private readonly FileStream _stream;

    public MyFileReader(FileStream stream)
    {
        _stream = stream;
    }

    public void Dispose()
    {
        _stream.Dispose();
    }
}

Currently in my program I Dispose objects like this.

        using (FileStream stream = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read))
        {
            using (MyFileReader reader = new MyFileReader(stream))
            {
                //...
            }
        }

This seems ok to me. later I have noticed That Classes are passed by reference so maybe if I dispose one of them there is no need to dispose the other one.

My question is Can i Do Something like this?

        using (FileStream stream = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read))
        {
            MyFileReader reader = new MyFileReader(stream);
            // Remove IDisposable from MyFileReader and stream will close after using.
        }

Or this one?

        FileStream stream = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read);
        // stream will close after using.
        using (MyFileReader reader = new MyFileReader(stream))
        {
            //...
        }

Solution

  • Yes, you can write code like that.

    But, no, you should not do that.

    Your class looks like on of XxxxReader classes that by convention own the stream they read from. As result your MyFileReader class is expected to dispose the inner stream. You also normally expected to dispose each and every disposable object when you know that such object's lifetime is over.

    Note that sometimes it lead to multiple Dispose calls on some objects (which should be expected by implementations of IDisposable). While it may sometimes lead to Code Analysis warning it is better than missing Dispose calls if one routinely tries to "optimize" number of calls to Dispose by skipping some.

    Alternative approach is to expose method that reads content which by convention is not expected to take ownership of stream/reader like:

     using(stream....)
     {
       var result = MyFileReader.ReadFrom(stream);
     }