Is it necessary to wrap StreamWriter in a using block?

Question

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?

Answer 1

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.

Answer 2

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.)