208

Classes such as Stream, StreamReader, StreamWriter etc implements IDisposable interface. That means, we can call Dispose() method on objects of these classes. They've also defined a public method called Close(). Now that confuses me, as to what should I call once I'm done with objects? What if I call both?

My current code is this:

using (Stream responseStream = response.GetResponseStream()) { using (StreamReader reader = new StreamReader(responseStream)) { using (StreamWriter writer = new StreamWriter(filename)) { int chunkSize = 1024; while (!reader.EndOfStream) { char[] buffer = new char[chunkSize]; int count = reader.Read(buffer, 0, chunkSize); if (count != 0) { writer.Write(buffer, 0, count); } } writer.Close(); } reader.Close(); } }

As you see, I've written using() constructs, which automatically call Dispose() method on each object. But I also call Close() methods. Is it right?

Please suggest me the best practices when using stream objects. :-)

MSDN example doesn't use using() constructs, and call Close() method:

Is it good?

Improve this question
7
  • If yo're using ReSharper you could define this as a "antipattern" within the patter catalog. ReSharper will mark each usage as error/hint/warning regarding to your definition. It's also possible to define how ReSharper has to apply a QuickFix for such an occurrence. Commented Sep 23, 2011 at 6:11
  • 3
    Just a tip: You can use the using statement like that for multiple disposable itens: using (Stream responseStream = response.GetResponseStream()) using (StreamReader reader = new StreamReader(responseStream)) using (StreamWriter writer = new StreamWriter(filename)) { //...Some code } Commented Sep 22, 2014 at 13:49
  • 1
    @TimothyGonzalez You do have to nest your using statements like that. using permits only one type, even if you are initializing multiple resources in the same statement. If you are using multiple statements or multiple types, by definition you must nest using statements; here, the objects are different types and must be in separate using statements. Commented Oct 12, 2017 at 14:46
  • 2
    @Suncat2000 You can have multiple using statements, but not nest them and instead stack them. I don't mean syntax like this which restricts the type: using (MemoryStream ms1 = new MemoryStream(), ms2 = new MemoryStream()) { }. I mean like this where you can redefine the type: using (MemoryStream ms = new MemoryStream()) using (FileStream fs = File.OpenRead("c:\\file.txt")) { } Commented Oct 12, 2017 at 15:17
  • 1
    @TimothyGonzalez Sorry to be pedantic but those latter statements are nested. Commented Oct 18, 2017 at 13:13

7 Answers 7

Reset to default
146

A quick jump into Reflector.NET shows that the Close() method on StreamWriter is:

public override void Close() { this.Dispose(true); GC.SuppressFinalize(this); }

And StreamReader is:

public override void Close() { this.Dispose(true); }

The Dispose(bool disposing) override in StreamReader is:

protected override void Dispose(bool disposing) { try { if ((this.Closable && disposing) && (this.stream != null)) { this.stream.Close(); } } finally { if (this.Closable && (this.stream != null)) { this.stream = null; /* deleted for brevity */ base.Dispose(disposing); } } }

The StreamWriter method is similar.

So, reading the code it is clear that that you can call Close() & Dispose() on streams as often as you like and in any order. It won't change the behaviour in any way.

So it comes down to whether or not it is more readable to use Dispose(), Close() and/or using ( ... ) { ... }.

My personal preference is that using ( ... ) { ... } should always be used when possible as it helps you to "not run with scissors".

But, while this helps correctness, it does reduce readability. In C# we already have plethora of closing curly braces so how do we know which one actually performs the close on the stream?

So I think it is best to do this:

using (var stream = ...) { /* code */ stream.Close(); }

It doesn't affect the behaviour of the code, but it does aid readability.

Improve this answer
Sign up to request clarification or add additional context in comments.

20 Comments

"In C# we already have plethora of closing curly braces so how do we know which one actually performs the close on the stream?" I don't think that this is a big problem: The stream is closed "at the right time", i.e., when the variable goes out of scope and is no longer needed.
Hmm, no, that is a "why the heck is he closing it twice??" speed bump while reading.
I disagree with the redundant Close() call. If someone less experienced looks at the code and doesn't know about using he will: 1) look it up and learn, or 2) blindly add a Close() manually. If he picks 2), maybe some other developer will see the redundant Close() and instead of "chuckling", instruct the less experienced developer. I'm not in favor of making life difficult for inexperienced developers, but I'm in favor of turning them into experienced developers.
If you use using + Close() and turn /analyze on, you get "warning : CA2202 : Microsoft.Usage : Object 'f' can be disposed more than once in method 'Foo(string)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 41" So while the current implementation is fine with calling Close and Dispose, according to documentation and /analyze, it's not ok and might change in future versions of .net.
+1 for the good answer. Another thing to consider. Why not add a comment after the closing brace like //Close or as I do, being a newbie, I add a one liner after any closing brace thats not clear. like for example in a long class I would add //End Namespace XXX after the final closing brace, and //End Class YYY after the second final closing brace. Is this not what comments are for. Just curious. :) As a newbie, I saw such code, hense why I came here. I did ask the question "Why the need for the second close". I feel extra lines of code dont add to clarity. Sorry.
|
59

No, you shouldn't call those methods manually. At the end of the using block the Dispose() method is automatically called which will take care to free unmanaged resources (at least for standard .NET BCL classes such as streams, readers/writers, ...). So you could also write your code like this:

using (Stream responseStream = response.GetResponseStream()) using (StreamReader reader = new StreamReader(responseStream)) using (StreamWriter writer = new StreamWriter(filename)) { int chunkSize = 1024; while (!reader.EndOfStream) { char[] buffer = new char[chunkSize]; int count = reader.Read(buffer, 0, chunkSize); if (count != 0) { writer.Write(buffer, 0, count); } } }

The Close() method calls Dispose().

Improve this answer

7 Comments

I'm pretty sure you don't need to be using the first responseStream since that is wrapped by the reader which will make sure its closed when the reader is disposed. +1 nontheless
This is confusing when you said The Close method calls Dispose... and in the rest of your post, you're implying that Dispose() would call Close(), I shouldn't call the latter manually. Are you saying they call each other?
Terrible answer. It assumes you can use a using block. I'm implementing a class that writes from time to time and therefore cannot.
@Jez Your class should then implement the IDisposable interface, and possibly also Close() if close is standard terminology in the area, so that classes using your class can use using (or, again, go for the Dispose Pattern).
The OP asked about properly closing stream objects. Not about some syntactic sugar.
|
29

For what it's worth, the source code for Stream.Close explains why there are two methods:

// Stream used to require that all cleanup logic went into Close(), // which was thought up before we invented IDisposable. However, we // need to follow the IDisposable pattern so that users can write // sensible subclasses without needing to inspect all their base // classes, and without worrying about version brittleness, from a // base class switching to the Dispose pattern. We're moving // Stream to the Dispose(bool) pattern - that's where all subclasses // should put their cleanup now.

In short, Close is only there because it predates Dispose, and it can't be deleted for compatibility reasons.

Improve this answer

Comments

22

The documentation says that these two methods are equivalent:

StreamReader.Close: This implementation of Close calls the Dispose method passing a true value.

StreamWriter.Close: This implementation of Close calls the Dispose method passing a true value.

Stream.Close: This method calls Dispose, specifying true to release all resources.

So, both of these are equally valid:

/* Option 1, implicitly calling Dispose */ using (StreamWriter writer = new StreamWriter(filename)) { // do something } /* Option 2, explicitly calling Close */ StreamWriter writer = new StreamWriter(filename) try { // do something } finally { writer.Close(); }

Personally, I would stick with the first option, since it contains less "noise".

Improve this answer

Comments

17

This is an old question, but you can now(C# 8.0) write using statements without needing to block each one. They will be disposed of in reverse order when the containing block is finished.

using var responseStream = response.GetResponseStream(); using var reader = new StreamReader(responseStream); using var writer = new StreamWriter(filename); int chunkSize = 1024; while (!reader.EndOfStream) { char[] buffer = new char[chunkSize]; int count = reader.Read(buffer, 0, chunkSize); if (count != 0) { writer.Write(buffer, 0, count); } }

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using

Improve this answer

2 Comments

Wow. Thanks for the warning! Talk about confusing. It was enough of an exercise looking up which block-end brace disposed the stream. Now we can guess if it can remain open until the end of the function? What were they thinking? Microsoft developers: creating sharp edges since the 20th century!
Heartily disagree. In the majority of cases this vastly improves readability (less indentation) and the disposal at the end of the containing scope remains deterministic. If you need disposal at a different point, using () {} is still there for you.
7

On many classes which support both Close() and Dispose() methods, the two calls would be equivalent. On some classes, however, it is possible to re-open an object which has been closed. Some such classes may keep some resources alive after a Close, in order to permit reopening; others may not keep any resources alive on Close(), but might set a flag on Dispose() to explicitly forbid re-opening.

The contract for IDisposable.Dispose explicitly requires that calling it on an object which will never be used again will be at worst harmless, so I would recommend calling either IDisposable.Dispose or a method called Dispose() on every IDisposable object, whether or not one also calls Close().

Improve this answer

1 Comment

FYI here's an article on the MSDN blogs that explains the Close and Dispose fun. blogs.msdn.com/b/kimhamil/archive/2008/03/15/…
-1

Just to complement other answers, as of C# 8.0, you don't need to open a block of code just to use an using statement

if (...) { using FileStream f = new FileStream(@"C:\users\jaredpar\using.md"); // statements } // Equivalent to if (...) { using (FileStream f = new FileStream(@"C:\users\jaredpar\using.md")) { // statements } }

docs:

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.