using statement FileStream and / or StreamReader - Visual Studio 2012 Warnings

JHubbard80 picture JHubbard80 · Aug 17, 2012 · Viewed 65.7k times · Source

The new Visual Studio 2012 is complaining about a common code combination I have always used. I know it seems like overkill but I have done the following in my code 'just to be sure'.

using (var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
    using (var sr = new StreamReader(fs))
    {
        // Code here
    }
}

Visual studio is 'warning' me that I am disposing of fs more than once. So my question is this, would the proper way to write this be:

using (var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
    var sr = new StreamReader(fs);
    // do stuff here
}

Or should I do it this way (or some other variant not mentioned).

var fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);

using (var sr = new StreamReader(fs))
{
    // Code here
}

I searched several questions in StackOverflow but did not find something that addressed the best practice for this combination directly.

Thank you!

Answer

Dan picture Dan · Aug 17, 2012

The following is how Microsoft recommends doing it. It is long and bulky, but safe:

FileStream fs = null;
try
{
    fs = new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
    using (TextReader tr= new StreamReader(fs))
    {
        fs = null;
        // Code here
    }
}
finally
{
    if (fs != null)
        fs.Dispose();
}

This method will always ensure that everything is disposed that should be despite what exceptions may be thrown. For example, if the StreamReader constructor throws an exception, the FileStream would still be properly disposed.