Is it best to pass an open SqlConnection as a parameter, or call a new one in each method?

Question

If methods/functions I'm going to call involve the need of an open SqlConnection, I will open this up in the method which is calling the function. For example:

protected static void btnSubmit(){
   conn.Open();
   myMethod(someParam, conn);
   conn.Close();
}

protected static void myMethod(object someParam, SqlConnection conn){
   //Some SQL commands etc here..
}

I do this so that I:

• Only ever open and close 1 SqlConnection per process

However, would it be better to structure my code like so:

protected static void btnSubmit(){
   myMethod(someParam);
}

protected static void myMethod(object someParam){
   SqlConnection conn = New SqlConnection(".....");
   conn.Open();
   //Some SQL commands etc here..
   conn.Close();
}

The advantage I see of structuring it this way is:

• I don't have to pass an extra parameter for each method
• If later down the line the method no longer has a SQL command, there is not an unused parameter being called each time

The disadvantage I see to this, is:

• If myMethod is a recursive method, then when it calls itself its going to be opening another SqlConnection, and so on, and so on..
• If btnSubmit is calling multiple methods which all require a SqlConnection, each one is going to open and close a new connection.

What is the best way of doing this, and which is most commonly practised?

Misbegotten

Answer 1

ADO.NET uses connection pooling, so it automatically reuses existing opened connections, even when you think that you are opening a new connection. Having that in mind, there is really no reason to pass a connection through your code (as a parameter). This will make your code much cleaner, with the same performance as when you were passing the connection as a parameter.

More details here

Also (and this is really important), please, use the "using" keyword. That way, you will not have to deal with closing the connection and cleanup, because your code as it is written now doesn't deal with closing the connections, so in a case of some exception you might end up with hitting connection limit on your server. Go with something like this:

using(var connection = new SqlConnection(<connection_string>))
{
  connection.Open();
  using(var command = connection.CreateCommand())
  {

  }
}

As you can see, there is no need to call connection.Close() or deal with exceptions and closing the connection in your finally block, because that is a "job" for the "using" block.

Also, one important note...transactions are not passed via connection polling, so if you want to keep your transaction across method calls, you will have to pass your connection (and that is the only reason I can think of why you should do that).

Brownell

So if I open 100 different SqlConnection objects, all for the same database/login, this is not putting any extra strain on the processing? – Misbegotten

Cheers for the extra details. So it's good practice to put my using statement in myMethod (using my example)? I've always avoided it, for performance purposes, but if it makes no difference it'll be so neater! – Misbegotten

If you don't believe it, make a test and try it :) You will see that there is no difference (at least not noticeable). There is a property in connection pool saying what is the max pool size (100 is a default value, I think), take a look here: msdn.microsoft.com/en-us/library/8xx3tyca(v=vs.71).aspx inside "Controlling Connection Pooling with Connection String Keywords" section – Brownell

I added a note regarding transactions because this is where it would make sense to pass the connection around. – Brownell

Thought I'd share my recent test results ... 10,000 items, requiring 10 calls to the database each, two seconds per call, on 2560 parallel threads, opened 1556 concurrent connections, and finished in less than 10 minutes. – Flagelliform

if I make a return inside the 'using' statement, either the inner one or the outer one, then will that be done safely? – Caribou

You say, "there is no need to ... deal with exceptions." But this is not true. Yes the using block will close connections, but it doesn't consume the Exception. The exception is still raised and they will still need to handle that somewhere in their program. – Yellowtail

Answer 2

The best pattern to use is Repository+UnitOfWork patterns.

So repository is created and passed the UnitOfWork which contains the connection. After work is done UnitOfWork is disposed.

// Pseudocode
using(UnitOfWork uow = new UnitOfWork())
{
   Repository.Init(uow);
   Repository.SaveInDb(stuff);
}

And Unit of work:

// PseudoCode
class UnitOfWork : IDisposable
{
   public UnitOfWork()
   {
      conn = new SqlConnection();
      conn.Open();
   }

   public void Dispose()
   {
       conn.Close();
   }

 ....

}

This is what I always use.

Some people prefer simpler approach where Repository owns connection. This is simpler but in case you need to have a transaction across multiple repositories, it will not work.

Halfbreed

+1 for even even slightly alluding to the fact that transaction boundaries should be a consideration when updating a database. – Estell

If you use TransactionScope it works perfectly well across multiple repositories as even if each repository has its own connection the connections will automatically enlist themselves to any TransactionScope that is already in effect. Is there any reason not to do it this way? – Balliol