Should a function be used in try block that has to be executed in finally block.

Question

try
{
  OpenConnection();
  RowsAffected = cmd.ExecuteNonQuery(); 
  CloseConnection(true); //should I use this function call here 
  //as well, when I am using it in finally 
  //block. For closing database connection.
}
catch (SqlException ex)
{ throw ex; }
finally
{ CloseConnection(true); }

Or Should I write it this way

try
{
  OpenConnection();
  RowsAffected = cmd.ExecuteNonQuery(); 
}
catch (SqlException ex)
{ throw ex; }
finally
{ CloseConnection(true); }

Answer 1

No, the finally block gets always executed, regardless of the success or failure of the code in the try block. In your first example the connection would be closed twice on success.

You say that you are checking for the connection state so that means you don't get an exception when closing the connection twice. Still, I think it is more appropriate to try to close it only when necessary.

Answer 2

Your finally block is always getting executed. Using Close functions like Dispose(), CLose() should be using in finally block

Answer 3

You should write in the second Way

                try 
                { 
                    OpenConnection(); 
                    RowsAffected = cmd.ExecuteNonQuery();  
                } 
                catch (SqlException ex) 
                { throw ex; } 
                finally 
                { CloseConnection(true); }

Answer 4

In this particular example, you can in fact do this:

using (var conn = new SqlConnection(...)) {
    // Do work
}

What the compiler does with that statement is essentially:

SqlConnection conn;
try {
  conn = new SqlConnection(...)
} finally {
  conn.Dispose();
}

Or thereabouts... the finally block is always exected, ensuring that a using block always calls Dispose.

The downside of this approach is that you can't catch the SqlException that could be thrown, so you end up doing something like this:

try {
  using (var conn = new SqlConnection(...)) {
    // Do work.
  }
} catch (SqlException ex) {

}

But the compiled code ends up representing:

try {
  SqlConnection conn;
  try {
    conn = new SqlConnection(...);
  } finally {
    conn.Dispose();
  }
} catch (SqlException ex) {

}

To be honest though it probably won't make any difference performance wise, as anytime you throw an exception, its going to be a performance hit. It does ensure that Dispose is always called.