tags:

views:

396

answers:

6

I've been told that System.IO.MemoryStream need not be wrapped in a using block because there is no underlying resource, this kinda goes against what i've always been told about streams ("if in doubt, use a using").

Is this true? Why then does MSDN example use one (summarized below)?

using(MemoryStream memStream = new MemoryStream(100))
{
            // do stuff
}
+5  A: 

A guideline is that if it implements IDisposable then you should dispose of it. You're just working with an API and are not aware of the implementation so you've no reason to not put it in a using block.

Charlie
+1  A: 

You do not lose anything by wrapping the memory stream in a using block, so if there's doubt, just do it. :)

Greg D
This can cause some problems with WCF services hosted in IIS as well as MVC. But I would suggest using "using" unless it causes problems.
Matthew Whited
Could you elaborate or provide a pointer to information on the problems with WCF/IIS and MVC?
Michael Burr
+2  A: 

If the class implements IDisposable, it should be disposed.

You don't need to bother guessing whether it does anything of significance or not - that's for the class to decide and implement. If the class doesn't do anything when the object gets disposed, then there's really no cost anyway, so there's no harm.

Encapsulation is one of the cornerstones of object oriented development. Why go out of your way to break that, especially if there's no good reason?

Michael Burr
+3  A: 

Having had a quick look in Reflector, it appears that Dispose on the MemoryStream does little other than to mark the stream as not being open, writeable or expandable.

Having said that, as a general approach, you should probably keep with the dispose pattern; escpecially as Streams all follow the "decorator" approach and should be easy to swap out for each other, or wrap one with another. Today's MemoryStream may get swapped out for another stream that does care tomorrow.

Rob Levine
+2  A: 

As Charlie said, it is a recommended practice to Dispose of all objects of classes that implements IDisposable.

If you look into the implementation of MemoryStream.Dispose(bool) (which is called by Stream.Dispose()), it becomes obvious you should be disposing of it since it updates a few control flags:

protected override void Dispose(bool disposing)
{
    try
    {
        if (disposing)
        {
            this._isOpen = false;
            this._writable = false;
            this._expandable = false;
        }
    }
    finally
    {
        base.Dispose(disposing);
    }
}
Alfred Myers
These flags are not a good reason to call Dispose in themselves. If it deallocated a resource, that would be a good reason. Marking the stream as non-writeable, when we have finished using it anyway buys you absolutely nothing.I agree that as a point of practice you should, but as a point of MemoryStream specifics, I disagree with your answer.
Rob Levine
I don't agree with your statement of "Marking the stream as non-writeable, when we have finished using it anyway buys you absolutely nothing.". Another part of the code could have a reference to the MemoryStream and try to write to it after you Dispose of it. You have to decide/check if the other code should really be writing to the MemoryStream. Depending on the outcome of the analysis, you code change the code that shouldn't be writing to the MemoryStream or move the Dispose elsewhere; or change the code to whatever makes sense in your scenario.
Alfred Myers
hang on - the OP is asking whether he has to wrap the stream in a "using". This it implies that his usage scenario is all in one method, so other code can't easily have a reference to it. That is my point - in this case it buys you nothing. If this was a class level stream - then that is a possibly a different matter, but it isn't in this use case.
Rob Levine
No, it may be a strong evidence, but it does not imply that his usage scenario is all in one method. "// do stuff" in his sample is open ended. You are assuming he does not pass the object to another part of the code (a member of another object which could be declared out of the scope of using) that could potentialy hold a reference and try to write to it after you have disposed it.
Alfred Myers
"//do stuff" in his method is all within the using statement, so it wouldn't matter if he passed it around as an argument to another method. Anyway - I'm really not trying to bait you, and yes, it is possible to conjure up usage scenarios where my assumption breaks down. I think we just disagree on whether setting those few internal flags is a compelling reason to call dispose, esp. for this usage scenario.
Rob Levine
+10  A: 

The C# idiom is that if an object implements IDisposable then you use a using block. This will allow all resources being used by the object be disposed of properly. You're not suppose to know the implementation details of MemoryStream. What you do know is that it implements IDisposable so you should dispose of it properly. Further, you think that you know now that it doesn't need to free any resources but how do you know that in the future MemoryStream won't change its underlying implementation so that it does use resources that need to be freed using Dispose? You don't, so since it implements IDispoable you have more future-proof code by just using the pattern now. Second, what if you change your code in the future to use a different type of Stream that does have managed resources? By wrapping the MemoryStream in a using block now you reduce maintenance issues in the future; again, this is the correct way to use objects, and specifically Streams that implement IDisposable. Third, it's a clear way to signal to readers that you are done using the object; it's what readers of your code will expect to see when they see you are using objects that implement IDisposable. Finally, this is the current implementation of MemoryStream.Dispose:

protected override void Dispose(bool disposing) {
    try {
        if (disposing) {
            this._isOpen = false;
            this._writable = false;
            this._expandable = false;
        }
     }
     finally {
         base.Dispose(disposing);
     }
}

So, this marks the stream as closed, not-writable and not-growable. If somehow someone else got a hold of a reference to the same MemoryStream, it now becomes unusable to them which could be a good thing. But this is really the least important issue; the implementation details don't matter.

Use a using block because MemoryStream implement IDispoable. Don't not use a using block because you think that MemoryStream doesn't have any resources that need to be freed.

Jason
I agree with this and so voted it up but would also mention please only wrap stuff in using blocks that you "own" in that you derived or created the stream in the method the using statement is in, nothing is worse than disposing of something three layers down, because eventually someone will try to read higher up from an already closed stream.
krystan honour