[Amendments made 10/26 - in blue]

Tonight I got to thinking.  I know, dangerous.

Anyway, my thoughts were revolving around the SqlCommand object and its companion the SqlConnection. These thoughts might also have been directed at their friends - the OleDb, Oracle, and Odbc variations, but I'll focus on the objects I use most.  Many of us developers want to write good, friendly, performant code, right?  Why, then, do so many of us fall back to the simple conveniences?  Take for example the following code:

public string DoSomething() {
   string sql = "SELECT Something FROM SOMEWHERE WHERE Anything = NothingAtAll";
   string cnStr = "Data Source=.;Initial Catalog=DB;Integrated Security=SSPI";
   using ( SqlCommand cm = new SqlCommand(sql, new SqlConnection(cnStr)) ) {
      cm.Connection.Open();
      cm.CommandType = CommandType.Text;
      return (string)cm.ExecuteScalar();
   }
}

What's wrong with the code?  Anything?  Well, for starters it's not friendly code.

I've seen code like this far too often; heck, I've even written something similar on occasion and invariably it comes back to bite me.  So, what's the issue?  Well this code is not friendly because it doesn't play nicely with the connection pool.  The connection that is passed into the SqlCommand's constructor is never closed.  It's hanging around long after the function returns waiting for the garbage collector to come around and clean it up so that the underlying connection can be returned to the pool.  This is definite madness - of all the resources you don't want to tie up any longer than necessary, database connections are some of the biggest.

You can write a real quick test harness to see how the connections are not being returned to the pool.  I've attached a simple one here (if you want the C# source, click here).  To effectively see the example, open your Performance Monitor, add a counter (SQLServer:General Statistics - User Connections).  I like to select the counter and make the line thicker so it stands out more.  Run the example and watch how the connections grows in increments of 15 (a number I arbitrarily chose in the example) for each new connection.  However, when you clean up your connection (that is close/dispose of it) you only gain one connection; it gets reused on each subsequent request indicative of the fact that it's being drawn from the pool.

So, I guess the real question is how do we properly clean up our connection?  We cannot always take the easy road and effectively and reliably insert a cm.Connection.Close() call before the end of our using block because if an exception is thrown preventing the code from reaching that line then we have a dangling connection still.  There are a few ways I can think of doing this, but to me the best most readable and elegant is this (best is a relative term that I may have misused):

public string DoSomething() {
   string sql = "SELECT Something FROM SOMEWHERE WHERE Anything = NothingAtAll";
   string cnStr = "Data Source=.;Initial Catalog=DB;Integrated Security=SSPI";

   using ( SqlConnection cn = new SqlConnection(cnStr) )
   using ( SqlCommand cm = new SqlCommand(sql, cn) ) {
      cn.Open();
      cm.CommandType = CommandType.Text;
      return (string)cm.ExecuteScalar();
   }
}

ASIDE:

One niceties of the C-family of languages is that if you have a block construct (e.g. an if(){...}) you can omit the braces if the block's execution path is a single statement or another block.  Therefore, you can place two, three, n using/fixed/checked/etc blocks in immediate succession and they are semantically nested.  A bonus is the editor will not indent them.  Personally, I don't like the syntactic appearance of

using () {
   using () {
   }
}

especially with all the other braces floating around.  Rather, I prefer the brevity and elegance of

using ()
using () {
}

Ok, enough of a soapbox.

In fact, there are a variety of means to ensure that the connection closes down that I failed to mention in the original post.  One is the case where your function returns a SqlDataReader -- though not always a preferred tactic especially if you don't have control over how the DataReader will be used.  In this case you cannot dispose of the SqlConnection (via using() or finally) or the connection will shut down prior to the DataReader being accessed.  Fortunately, you can pass a parameter to the SqlCommand.ExecuteReader() method of CommandBehavior.CloseConnection which will take care of the connection once the connection is gone.  Again, if you're using this strategy of returning a SqlDataReader your architecture might very well be flawed - consider instead calling back via a delegate which would effectively lend the reader to a function.  This gives you much better lifetime management on your connections and can help keep you in control.

I suppose, therefore, that it's worth remembering to pay special attention to your connections and DISPOSE of them - if nothing else, at least CLOSE THEM....  This can be of utmost importance on an ASP.NET application where you have client request flowing in and you want to maximize the effectiveness the connection pool.