views:

280

answers:

4

How can I make sure in the following code snippet that IDataReader is disposed of if ExecuteReader throws an exception?

using (IDataReader rdr = cmd.ExecuteReader())
{
    // use it
}

It makes sense to me that the using syntatic sugar does not call Dispose (since there is no instance to call it on). However, how can I be sure that the scarce resources that are normally allocated by classes that implement IDisposable will be releases?

A: 

I thought that the using statement was translated to the IL that is similar to:

try
{
}
finally
{
    disposableUsedObject.Dispose();
}

So, I would think that in normal circumstances, the Dispose should be called ?

Also, you should not throw exceptions inside the constructor , as users will not expect that an exception will be thrown when they instantiate an object. I would move the initialization logic that can throw an exception to another method (Initialize) that has to be called after instantiating the object.

Frederik Gheysels
But if the constructor throws the exception, there is no object to dispose?
ck
@ck Exactly my problem
jpoh
You should not throw exceptions in a constructor, as this is a bit unexpected. Users of your class do not expect that an exception can be thrown inside a constructor / when instantiating an object. :)I think it is better to move that initialization logic (that can trhow an exception to another method (Initialize, or something like that)
Frederik Gheysels
Apologies, my contrived example appears to have made this question confusing. I'll edit the question so that its clearer.
jpoh
ck is correct. I tested this using simple Console application and dispose will not get called when the error is thrown from the constructor.
Jimmy Chandra
@Jimmy: Dispose will not be called because there is no object to Dispose!
John Saunders
+11  A: 

If ExecuteReader, in your example, throws an exception it never returns anything. It is then up to the implementation of ExecuteReader to dispose of anything created before the exception.

Simon Svensson
Right, so it becomes the responsibility of the thrower of the exception to clean up if an exception is thrown?
jpoh
@jpoh - it **has** to be; the caller doesn't have any objects to try to clean up. At worst, the finalizer will kick in.
Marc Gravell
+3  A: 

If the constructor of an object fails to run then you do not have an object that needs to be disposed.

If you are writing a constructor which could throw an exception, you had better make sure you clean up anything you need to by using using or try-catch block.

In your IDataReader example, it is sufficient to simply dispose of the command object if the cmd.ExecuteReader() method call fails.

Matt Howells
Just to clarify what Matt said, use using / try catch inside the constructor of your disposable class to have it properly disposed its own internal resources. I'm leaning more toward try catch since the resource might be a class level resource and not just constructor level. If it is, then using is more appropriate.
Jimmy Chandra
A: 

How about moving the code that does the initialization outside the constructor into a separate function. In essence you'll have

using (var tc = new TestClass())
{
    tc.Initialize();  // setup the object. acquire expensive resources here, etc.

    ..more code ..
}
This is not necessary. Again, if the constructor throws, then there is nothing to clean up.
John Saunders