dgmulf
dgmulf

Reputation: 485

C# - nested using statements redundant?

I came across the following example code in the MSDN documentation, demonstrating the use of the System.IO.StreamReader class to read UTF-8 text from a System.IO.FileStream object. The two nested using statements struck me as redundant - surely calling Dispose() on one of the objects does the trick, and properly releases the file handle? (Source: http://msdn.microsoft.com/en-us/library/yhfzs7at.aspx)

using (FileStream fs = new FileStream(path, FileMode.Open)) 
{
    using (StreamReader sr = new StreamReader(fs)) 
    {

        while (sr.Peek() >= 0) 
        {
            Console.WriteLine(sr.ReadLine());
        }
    }
}

Would it not be simpler, and equally correct, to rewrite that code in the following way?

using (FileStream fs = new FileStream(path, FileMode.Open)) 
{
    StreamReader sr = new StreamReader(fs);

    while (sr.Peek() >= 0) 
    {
        Console.WriteLine(sr.ReadLine());
    }
}

Upvotes: 3

Views: 862

Answers (2)

Henk Holterman
Henk Holterman

Reputation: 273314

The second sample is simply wrong on principle.

It won't leak anything but that relies on the knowledge that a StreamReader has no resources of its own and does not actually needs Disposing even though it is IDisposable.

A single using(){} around the StreamReader would have been more or less correct here, based on the documented (and criticized) feature that the Reader will close its Stream.

The best practice here is to use 2 using statments. Note that they are very cheap, and you simply want code that's consistent.

Upvotes: 3

Alexander Gessler
Alexander Gessler

Reputation: 46627

According to the documentation, The StreamReader object calls Dispose() on the provided Stream object when StreamReader.Dispose is called. This means using the StreamReader guarantees disposal of the underlying Stream. It does not hold vice versa: disposing only the Stream does not suffice - the StreamReader may be allocating other native resources. So the second sample is not correct.

(using only the StreamReader does not cover the case that the StreamReader constructor can throw. To cover this case, both using's would be needed. Since it only throws for non-readable or null streams, this may not be relevant though.)

In general, you should always dispose on every disposable object. It is part of the IDisposable contract that it does not hurt to dispose an object multiple times, and the overhead of doing so is low.

Upvotes: 3

Related Questions