What is the right way to use multiple using statements with possible exception?

Question

I'm working with SqlConnection, SqlDataAdapter and SqlCommand.

I was just wondering if the following code is correct and what is the better way if possible to do this.

private void FillWithData(int id)
{
    try
    { 
       LoadTable1(id);
    }
    catch(Exception ex){throw ex}
}

private void LoadTable1(SqlConnection conn, int id)
{
   try
   {
       using (conn = new SqlConnection(sConnectionString)) 
       using (SqlDataAdapter table1DA = new SqlDataAdapter ())
       {
          DataTable tblTable1 = Table1;
          SqlCommand table1CMD = getSelectTable1(conn, id);
          table1DA.SelectCommand = table1CMD;
          table1DA.Fill(tblTable1);
          table1CMD.Dispose();
       }
   }
   catch(SqlException ex)
   {
       if (conn.State == ConnectionState.Open) conn.Close();
       conn.Dispose();
       throw ex;   
   }
   finally
   {
       try
       {
          if (conn.State == ConnectionState.Open) conn.Close();
       }
       catch (Exception){}
       conn.Dispose();
   }         
}

I'm going to have some more methods that do the same for other tables as well. Currently everything is executed under one method that has try/catch block inside of which SqlDataAdapter and SqlCommands for each table are defined. My goal is to separate logic for different tables into different methods

Answer 1

I see a few possible improvements:

1. Do not swallow exceptions (catch (Exception){}) - either handle it or just let it bubble up.
2. You do not need to close or dispose of the connection - the using block will do that for you
3. You should put the SqlCommand in a using block as well.
4. You are passed in a connection but then overwrite the local variable - the connection that is passed in is not changed. Why have a parameter at all?
5. throw ex; is not a best practice since you lose the original stack trace - better to just throw; instead. Since you aren't doing anything in the catch block except closing the connection (which is unnecessary) the catch block can be removed altogether.

Making those changes would leave you with just:

private void FillWithData(int id)
{
    LoadTable1(id);
}
private void LoadTable1(int id)
{
   using (SqlConnection conn = new SqlConnection(sConnectionString)) 
   using (SqlDataAdapter table1DA = new SqlDataAdapter ())
   using (SqlCommand table1CMD = getSelectTable1(conn, id))
   {
      DataTable tblTable1 = Table1;
      table1DA.SelectCommand = table1CMD;
      table1DA.Fill(tblTable1);
   }
}

And since SqlDataAdapter has a constructor that takes a SQlCommand you can just do:

private void LoadTable1(int id)
{
   using (SqlConnection conn = new SqlConnection(sConnectionString)) 
   using (SqlCommand table1CMD = getSelectTable1(conn, id))
   using (SqlDataAdapter table1DA = new SqlDataAdapter (table1CMD))
   {
      table1DA.Fill(Table1);
   }
}