views:

200

answers:

4

If I have code with nested objects like this do I need to use the nested using statements to make sure that both the SQLCommand and the SQLConnection objects are disposed of properly like shown below or am I ok if the code that instantiates the SQLCommand is within the outer using statement.

using (var conn = new SqlConnection(sqlConnString))
{
    using (var cmd = new SqlCommand())
    {
        cmd.CommandType = CommandType.Text;
        cmd.CommandText = cmdTextHere;
        conn.Open();

        cmd.Connection = conn;
        rowsAffected = cmd.ExecuteNonQuery();
    }
}
+7  A: 

Yes. You could clean it up a little like this

using (SqlConnection conn = new SqlConnection(sqlConnString))
using (System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand())
{
   // code here
}

But you would still want a using for each IDisposable object.

Edit: Consider this example of not using an inner using statement.

class A : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("A Disposed");
    }
}

class B : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("B Disposed");
    }
}

Code

using (A a = new A())            
{
    B b = new B();
}

At the end of the block, A is properly disposed. What happens to B? Well, it's out of scope and simply waiting to be garbage collected. B.Dispose() is not called. On the other hand

using (A a = new A())
using (B b = new B())
{
}

When execution leaves the block (actually, blocks), the compiled code executes calls to the Dispose() method of each object.

Anthony Pegram
Any particular reason why make it always explicit? This answers deviates from question's real purpose.
Nayan
@Nayan, see my comment to your answer.
Anthony Pegram
+3  A: 

You should call dispose on both, however, it is easier to read if you just do this:

using (SqlConnection conn = new SqlConnection(sqlConnString)) 
using (SqlCommand cmd = new SqlCommand()) 
{ 
     cmd.CommandType = CommandType.Text; 
     cmd.CommandText = cmdTextHere; 
     conn.Open(); 

     cmd.Connection = conn; 
     rowsAffected = cmd.ExecuteNonQuery(); 
} 

since there's an implicit block created (like an if statement or for statement) and since using is an entire block, you don't need the braces and it looks neater in my opinion. Some will say you shoul always use braces because it's too easy to accidentally add a second statement and create a bug, but in this case i think that's unlikely.

Mystere Man
Neat, yes. But it still not answers the question. :/
Nayan
Yes, it does. You should call dispose on both, thus the using statement on both.
Mystere Man
A: 

To answer your question, yes, you can do that.

Since the SqlCommand object is limited by outer braces, it will be collected by GC when the execution goes out of the block.

Other answers are also okay, but they did not exactly answered your question :)

Nayan
Garbage collection is not the same as calling .Dispose(). Further, the GC is indeterminate, you do not know precisely when the object will be collected.
Anthony Pegram
Ok, understood. Thanks!
Nayan
Relying on GC to collect SqlCommand instance can cause very nasty memory "leaks", leaving many instances in memory.
Marek
@Nayan: I agree with Marek. Please read my answer. This is the long version of Marek's answer :-).
Steven
+2  A: 

You can omit the using around the SqlCommand. The GC will eventually clean it up for you. However, I strongly advice you not to do this. I will explain why.

SqlCommand indirectly inherits from System.ComponentModel.Component, and it therefore inherits its Finalizer method. Not calling dispose on a SqlCommand will ensure that the command is promoted at least one generation after it goes out of scope (the .NET garbage collector is a generational gc). For instance: when the command was in gen 1, it will move on to gen 2. Finalizable objects are kept longer in memory to ensure the finalizer can safely run. But not only the command itself is kept in memory, but all the objects it references go with it to that generation. Objects that it will reference are the SqlConnection, the list of SqlParameter objects, the possibly large CommandText string, and many other internal object it references. That memory can only be removed when that generation is collected, but the higher the generation the less often it is swept.

Not calling will therefore give extra memory pressure and extra work for the finalizer thread.

When .NET is unable to allocate new memory, the CLR will force a garbage collect of all generations. After this, the runtime will usually have again enough space to allocate new objects. However, when this forced collect happens when there are a lot of objects in memory that still have to be promoted to a next gen (because they are finalizable, or are referenced by a finalizable object) it is possible that the CLR is unable to free up enough memory. An OutOfMemoryException will be the result of this.

I must admit I have never seen this happen, because developers didn’t dispose just their SqlCommand objects. However, I have seen OOMs in production systems a lot, caused by not disposing objects properly.

I hope this gives a little bit of background on how the GC works and what the risk is of not disposing (finalizable) object properly. I always dispose all disposable objects. While a look in Reflector could prove that this not necessarily for a certain type, this kind of programming leads to code that is less maintainable and makes code depend on internal behavior of a type (and this behavior might change in the future).

Steven
Thanks for explanation, Steven!
Nayan