views:

193

answers:

4

I noticed This question, but my question is a bit more specific.

Is there any advantage to using

using (SqlConnection conn = new SqlConnection(conStr))
{
     using (SqlCommand command = new SqlCommand())
     {
        // dostuff
     } 
}

instead of

using (SqlConnection conn = new SqlConnection(conStr))
{
     SqlCommand command = new SqlCommand();
     // dostuff
}

Obviously it does matter if you plan to run more than one command with the same connection, since closing an SqlDataReader is more efficient than closing and reopening a connection (calling conn.Close();conn.Open(); will also free up the connection).

I see many people insist that failure to close the SqlDataReader means leaving open connection resources around, but doesn't that only apply if you don't close the connection?

A: 

Isn't it just a matter of you freeing the resource now vs. garbage collection freeing it later?

John at CashCommons
I think relying on the garbage collector to free SqlConnections is a dangerous thing to do.
Brian
Sounds reasonable ... I wasn't making a judgment as to which was better, just stating what I saw the difference to be.
John at CashCommons
A: 

It may not be necessary in a lot of cases, but it's best practice for a reason. There's no reason to let resources persist beyond the point where they are no longer useful. The using construct helps ensure this.

Daniel DiPaolo
This is not really an answer to my question. My point was that in my case this specific issue does not apply, since the relevant resources are freed anyways. This is not merely an implementation detail; it's a documented implementation detail.
Brian
+2  A: 

Technically it's not needed; closing a SqlConnection should destroy any resources that the SqlDataReader is using. The reverse is also true; you don't need to Dispose the SqlConnection if you dispose a SqlDataReader that was created with CommandBehavior.CloseConnection.

Practically speaking, though, when a class implements IDisposable, you should Dispose it when you're finished with it. The implementation details of framework classes are subject to change at any time, and unless the documentation specifically outlines circumstances under which it is not necessary to Dispose the instance, there is always a possibility that some future change/update will result in your code having a resource leak.

It's really no extra effort - so just wrap it in a using block.

Aaronaught
A good point, though a future change breaking this is unlikely (and the freeing of the resource involved *is* a documented piece of functionality).
Brian
As an aside, there are a few weird cases where wrapping classes with in a using block will cause code issues. My recollection of this is poor, but I think it had to do with COM components and an issue which some people considered to be a bug.
Brian
@Brian: It's not documented anywhere that closing a `SqlConnection` frees all resources associated with a `SqlDataReader`. It closes the connection - obviously - but you don't know for sure that `SqlDataReader` doesn't use any other resources. As for those weird cases, my guess is that you're talking about generated WCF clients, which have a broken `IDisposable` implementation.
Aaronaught
+5  A: 

In my opinion, there are two rules to follow here:

  1. Classes that implement IDisposable should be wrapped in a using block.
  2. You should not rely on a class's implementation of IDisposable to ignore rule 1.

That is, even if you know that disposing the connection object took care of disposing its associated command object, you should not rely on this behavior.

By the way, it's possible to nest using blocks in a cleaner fashion:

using (SqlConnection conn = new SqlConnection(conStr))
using (SqlCommand command = new SqlCommand())
{
    // dostuff
}

and I would use

SqlCommand command = conn.CreateCommand();

instead of creating a new SqlCommand and then associating it with the connection.

Jamie Ide
Exactly as I would have put it.
Chris