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?

Community
  • 1
  • 1
Brian
  • 25,523
  • 18
  • 82
  • 173

5 Answers5

16

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
  • 48,427
  • 16
  • 81
  • 117
5

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
  • 120,909
  • 25
  • 266
  • 342
  • 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 Apr 16 '10 at 21:38
  • 1
    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 Apr 16 '10 at 21:50
  • @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 Apr 17 '10 at 01:37
1

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
  • 55,313
  • 14
  • 116
  • 115
  • 1
    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 Apr 16 '10 at 21:37
0

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

John
  • 15,990
  • 10
  • 70
  • 110
  • I think relying on the garbage collector to free SqlConnections is a dangerous thing to do. – Brian Apr 16 '10 at 21:45
  • Sounds reasonable ... I wasn't making a judgment as to which was better, just stating what I saw the difference to be. – John Apr 16 '10 at 22:23
  • Garbage collector does not free-up or clean-up unmanaged resources, which is the main purpose of the IDisposable interface. – tomosius Sep 22 '15 at 05:47
0

There's a difference. If you create 2 of those readers without opening/closing the connection, but don't dispose of the first one before using the second, you'll get a conflict saying the connection is already associated to an open reader.

Jerod Venema
  • 44,124
  • 5
  • 66
  • 109