Jon Skeet wrote:
[snip]
I think it would be a good idea to do so though. It can't do any harm,
and in fact it's likely to make garbage collection quicker, as it can
unregister the instance for finalization.
I agree that you'd better call Dispose() on types with finalizers if you
want the best performance (both memory footprint and speed). Because of this
fact and also because I hadn't yet grocked all the implications of GC, I was
a very strong advocate of the "Call Dispose() whenever possible" policy
until recently.
But, with a few thousand lines of experience behind me, I slowly but surely
come to the conclusion that following this policy often clutters your code
more than anything else (>99% of the code I write is not
performance-critical). I even think that I introduced more bugs this way.
The following real world code, which I have written today, makes for a good
example:
public void Log( string eventLogMsg )
{
MemoryStream stream = new MemoryStream();
// add eventLogMsg to stream and reset position
long origin = stream.Position;
StreamWriter writer = new StreamWriter( stream );
writer.Write( eventLogMsg );
writer.Flush();
stream.Position = origin;
// read stream into DataSet
StreamReader reader = new StreamReader( stream );
DataSet dataSet = new DataSet();
dataSet.ReadXml( reader );
// ...
}
eventLogMsg contains an XML message, which I need to read into dataSet,
which is then used for further processing. While I might have missed easier
ways to achieve the same, I do claim that this is perfectly reasonable code.
Two months ago, I would have written the same function as follows:
public void Log( string eventLogMsg )
{
using( MemoryStream stream = new MemoryStream() )
{
// add eventLogMsg to stream and reset position
long origin = stream.Position;
using ( StreamWriter writer = new StreamWriter( stream ) )
{
writer.Write( eventLogMsg );
}
stream.Position = origin; // BOOM
// read stream into DataSet
DataSet dataSet = new DataSet();
using( StreamReader reader = new StreamReader( stream ) )
{
dataSet.ReadXml( reader );
}
// ...
}
}
Problem is, this function will invariably throw an exception when it tries
to reset the Stream.Position property, which is not immediately obvious
unless you know that StreamWriter.Dispose() also calls Close() on the stream
object we passed to its constructor.
Sure, there are situations where you absolutely need to call Dispose() or
your program would simply not behave as expected (as was discussed here:
http://groups.google.com/[email protected]).
However, I believe these situations are more rare than the ones where a
naive programmer could introduce a bug with calling Dispose() too early
(Jeffrey Richter in his excellent book "Applied .NET framework programming
in C sharp" voices similar concerns).
Regards,
Andreas
