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?

Answer 1

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.

Answer 2

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).