Eric
Eric

Reputation: 19152

Is it necessary to wrap StreamWriter in a using block?

A few days ago I posted some code like this:

StreamWriter writer = new StreamWriter(Response.OutputStream);
writer.WriteLine("col1,col2,col3");
writer.WriteLine("1,2,3");
writer.Close();
Response.End();

I was told that instead I should wrap StreamWriter in a using block in case of exceptions. Such a change would make it look like this:

using(StreamWriter writer = new StreamWriter(Response.OutputStream))
{
    writer.WriteLine("col1,col2,col3");
    writer.WriteLine("1,2,3");
    writer.Close(); //not necessary I think... end of using block should close writer
}
Response.End();

I am not sure why this is a valuable change. If an exception occurred without the using block, the writer and response would still be cleaned up, right? What does the using block gain me?

Upvotes: 26

Views: 19911

Answers (10)

Joe White
Joe White

Reputation: 97768

I'm going to give the dissenting opinion. The answer to the specific question "Is it necessary to wrap StreamWriter in a using block?" is actually No. In fact, you should not call Dispose on a StreamWriter, because its Dispose is badly designed and does the wrong thing.

The problem with StreamWriter is that, when you Dispose it, it Disposes the underlying stream. If you created the StreamWriter with a filename, and it created its own FileStream internally, then this behavior would be totally appropriate. But if, as here, you created the StreamWriter with an existing stream, then this behavior is absolutely The Wrong Thing(tm). But it does it anyway.

Code like this won't work:

var stream = new MemoryStream();
using (var writer = new StreamWriter(stream)) { ... }
stream.Position = 0;
using (var reader = new StreamReader(stream)) { ... }

because when the StreamWriter's using block Disposes the StreamWriter, that will in turn throw away the stream. So when you try to read from the stream, you get an ObjectDisposedException.

StreamWriter is a horrible violation of the "clean up your own mess" rule. It tries to clean up someone else's mess, whether they wanted it to or not.

(Imagine if you tried this in real life. Try explaining to the cops why you broke into someone else's house and started throwing all their stuff into the trash...)

For that reason, I consider StreamWriter (and StreamReader, which does the same thing) to be among the very few classes where "if it implements IDisposable, you should call Dispose" is wrong. Never call Dispose on a StreamWriter that was created on an existing stream. Call Flush() instead.

Then just make sure you clean up the Stream when you should. (As Joe pointed out, ASP.NET disposes the Response.OutputStream for you, so you don't need to worry about it here.)

Warning: if you don't Dispose the StreamWriter, then you do need to call Flush() when you're done writing. Otherwise you could have data still being buffered in memory that never makes it to the output stream.

My rule for StreamReader is, pretend it doesn't implement IDisposable. Just let it go when you're done.

My rule for StreamWriter is, call Flush where you otherwise would have called Dispose. (This means you have to use a try..finally instead of a using.)

Upvotes: 17

to StackOverflow
to StackOverflow

Reputation: 124746

While it's good practice to always dipose disposable classes such as StreamWriter, as others point out, in this case it doesn't matter.

Response.OutputStream will be disposed by the ASP.NET infrastructure when it's finished processing your request.

StreamWriter assumes it "owns" a Stream passed to the constructor, and will therefore Close the stream when it's disposed. But in the sample you provide, the stream was instantiated outside your code, so there will be another owner who is responsible for the clean-up.

Upvotes: 0

n8wrl
n8wrl

Reputation: 19765

My rule of thumb is, if I see Dispose listed in intellisense, I wrap it in a using block.

Upvotes: 1

Fredrik Mörk
Fredrik Mörk

Reputation: 158349

Wrapping the StreamWriter in a using block is pretty much equivalent of the following code:

StreamWriter writer;
try
{
    writer = new StreamWriter(Response.OutputStream);
    writer.WriteLine("col1,col2,col3");
    writer.WriteLine("1,2,3");
}
catch
{
    throw;
}
finally
{
    if (writer != null)
    {
        writer.Close();    
    }
}

While you could very well write this code yourself, it is so much easier to just put it in a using block.

Upvotes: 3

Adam Wright
Adam Wright

Reputation: 49386

Eventually, the writer will be cleaned up. When this happens is up to the garbage collector, who will notice that the Dispose for the command has not been called, and invoke it. Of course, the GC may not run for minutes, hours or days depending on the situation. If the writer is holding an exclusive lock on say, a file, no other process will be able to open it, even though you're long finished.

The using block ensures the Dispose call is always made, and hence that the Close is always called, regardless of what control flow occurs.

Upvotes: 3

John Saunders
John Saunders

Reputation: 161791

In almost every case, if a class implements IDisposable, and if you're creating an instance of that class, then you need the using block.

Upvotes: 0

Robert
Robert

Reputation: 3062

the using block calls dispose() when it ends. It's just a handy way of ensuring that resources are cleaned up in a timely manner.

Upvotes: 0

Jamie Ide
Jamie Ide

Reputation: 49271

In my opinion it's necessary to wrap any class that implements IDisposable in a using block. The fact that a class implements IDisposable means that the class has resources that need to be cleaned up.

Upvotes: 1

Nick
Nick

Reputation: 1718

If an exception occurs without the using block and kills the program, you will be left with openconnections. The using block will always close the connection for you, similiar to if you were to use try{}catch{}finally{}

Upvotes: 3

kemiller2002
kemiller2002

Reputation: 115508

Nope the stream would stay open in the first example, since the error would negate the closing of it.

The using operator forces the calling of Dispose() which is supposed to clean the object up and close all open connections when it exits the block.

Upvotes: 24

Related Questions