Consider the following code:
using (var ms = new MemoryStream())
{
using(var writer = BinaryWriter(ms))
{
writer.Write(/*something*/);
writer.Flush();
}
Assert.That(ms.Length > 0); // Throws ObjectDisposedException
}
On the one hand, a disposable object sho开发者_C百科uld dispose of it's resources; I get that, but on the other hand, the object didn't create and doesn't own this resource, it was provided -> calling code should take responsibility for it... no?
I can't think of any other situations like this, but is it a consistent pattern in the framework for any class receiving disposable objects to dispose of them on its own dispose?
There is an implicit assumption that you will only have one writer per stream, so the writer assumes ownership of the stream for convenience - you then obly have one thing to clean up.
But I agree; this is not always true, and often inconvenient. Some implementations (DeflateStream, GZipStream, for example) allow you to choose. Otherwise the only real option is to inject a dummy stream between the writer and the underlying stream; IIRC there is a NonClosingStreamWrapper in Jon Skeet's "MiscUtil" library that does exactly this: http://www.yoda.arachsys.com/csharp/miscutil/
Usage would be something like:
using (var ms = new MemoryStream())
{
using(var noClose = new NonClosingStreamWrapper(ms))
using(var writer = BinaryWriter(noClose))
{
writer.Write(/*something*/);
writer.Flush();
}
Assert.That(ms.Length > 0);
}
I totally agree with you. This is not consistent behavior but it is how it has been implemented. There are comments at the end of the documentation about this behavior which is not very intuitive. All stream writers just take ownership of the underlying stream and dispose it. Personally I always nest my using
statement like this:
using (var ms = new MemoryStream())
using(var writer = BinaryWriter(ms))
{
writer.Write(/*something*/);
}
so that a code like the one you put in the Assert shouldn't be written.
The right thing to have done would have been to have a constructor parameter for the streamwriter indicate whether the stream should be disposed when the constructor is. Given that Microsoft didn't do that, it may be good to define a NonDisposingStream(Of T as Stream) class which wraps a stream but does not pass a Dispose call to the wrapped stream. One could then pass a new NonDisposingStream to the constructor of a StreamWriter, and the underlying stream would be safe from disposal (it would be necessary, of course, to dispose of the stream yourself).
Having an object which can dispose of a passed-in object is useful. While such behavior doesn't coincide with the usual pattern of an object's creator handling its disposal, there are often situations where an object's creator will have no idea how long the object will actually be needed. For example, a method may be expected to create a new StreamWriter which uses a new Stream. The owner of the StreamWriter will know when it should be disposed, but may not know of the inner stream's existence. The creator of the inner stream will have no idea how long the outer StreamWriter will be used. Having ownership of the stream "handed off" to the StreamWriter solves the disposal problem in that particular (common) case.
I propose this wrapper class:
public class BetterStreamWriter : StreamWriter
{
private readonly bool _itShouldDisposeStream;
public BetterStreamWriter(string filepath)
:base(filepath)
{
_itShouldDisposeStream = true;
}
public BetterStreamWriter(Stream stream)
: base(stream)
{
_itShouldDisposeStream = false;
}
protected override void Dispose(bool disposing)
{
base.Dispose(disposing && _itShouldDisposeStream);
}
}
Objects should not dispose stuff they didn't instantiate. If it's a file stream writer, it should dispose. If it's an external stream, it shouldn't.
Shouldn't have implemented opening file path in the first place. This violate single responsibility principle as the object both manages writing and lifetime of the file.
精彩评论