tags:

views:

64

answers:

3

I would like to create a method that returns an XmlReader. Depending on the circumstances the XmlReader may be fed different types of streams, either a StringReader or a MemoryStream.

Normally I dispose a StringReader or MemoryStream with a using block, but since I want to return an XmlReader instead, I cannot do that if I want to go with this design. I don't expect the MemoryStream to allocate huge amounts of memory, so I can live with a slight delay in resource deallocation.

Are the consequences of letting the GC dispose StringReader and MemoryStream acceptable in this case?

I should clarify that this is a practical question not a best-practice question. Obviously the theory dictates that I should clean up my own resource allocations, but theory also says that I should prefer the simplest possible design for maximum maintainability. In some cases breaking best-practice can IMHO be justified and my question is whether this concrete case justifies breaking best-practice.

Also this is only about StringReader and MemoryStream, not a general stream or reader. My reason for justifying it in this case is that the actual creation of the StringReader / MemoryStream is well encapsulated in the method that returns the XmlReader, hence it is under control that the XmlReader will not be fed a stream with a limited resource.

+1  A: 

Answer: NO. You should always dispose disposable resources. In the case of returning a stream from a method, the disposal should be done by the caller.

Darin Dimitrov
In this case I do not return a stream directly since it is wrapped in the XmlReader. Can you explain why I should _always_ dispose disposable resources? After all the GC will eventually do that for me and in some cases it is rather impractical to dispose it manually.
Holstebroe
@Holstebroe: You *don't know* that the GC will eventually do that for you. You may be able to use Reflector/ILDASM and see that a particular implementation *either* has no unmanaged resources *or* has a fallback finaliser to clean them up, but you can't guarantee that this will be the case across all implementations (including those that don't yet exist).
LukeH
@Holstebroe, if an exception is thrown you will leak unmanaged handles. This could be catastrophic in ASP.NET application.
Darin Dimitrov
+1  A: 

If you allocate a resource for the explicit internal use of your class, then your class owns the responsibility of disposing of that resource. If you are allocating a resource on behalf of a caller, than it is the responsibility of the caller to manage the lifetime of the resources requested.

While the CLR will eventually free resources alliocated by any object, no garentees are made as to when a specific object will be collected (deallocated).

Therefore if an object uses a relatively scarse resource such as a file handle and that object is not disposed of by the code that created it, the file handle will remain unavailable for use by the system or other applications until the garbage collector collects the object holding the handle.

On a single users desktp[ machine, it is unlikely that you would run ouut of file handles, but on a busy server it is more likely that you will approach the maximum number of file handles available, and the closer one gets to exhausting a system resource the more likely it is that the machie will experience performance degradation, so the timely release of resources becomes a much bigger concern.

Crippledsmurf
I understand and respect that in general, but this is a practical question more than theoretical. I mean after all the MemoryStream / StringReader may only reserve a tiny amount of memory and my question is if that justifies breaking the theoretical best-practice.
Holstebroe
Can you be certain that in every possible situation and environment that exists from now until the end of your software's life that the MemoryStream and StringReader will only ever contain a small amount of data? Even if you can, I still think that as a developer one should minimise the footprint of an application on the system as much as possible, even if it;'s only a small amount.
Crippledsmurf
+3  A: 

Nothing is going to suffer in this case, but IMO it is still very bad practice. You own them - why not do it properly? Actually, disposing a MemoryStream still can't deallocate etc - it is still bound by GC. But there is a distinct code smell in here somewhere. This can become real problems if anything ever changes, and suddenly it isn't a MemoryStream but something else, etc.

We can't make you dispose it, but personally: I am fastidious about my usings

Marc Gravell
+1: Agreed, if you constructed it, you own it, and it implements IDisposable, I'm religious about disposing that object. It's my responsibility, I need a very good reason for deviating from the expectations.
Lasse V. Karlsen
If you are using XslCompiledTransform you often end up triple nesting your usings, which can sometimes be annoying when you want to introduce some flexibility in the choice of readers / streamers in the simple cases, where it's just wrapped strings or byte arrays.
Holstebroe