ADO.NET and Disposing without Using

Question

I have a project that isn't using USING anywhere with their ADO.NET code. I am cleaning up their unclosed connections. Is the below code a best practice with try/catch/finally. I also have some that contains SqlTransaction that I'm disposing in between the command and connection dispose.

SqlConnection con = new SqlConnection(ConfigurationManager.AppSettings["MyNGConnectDashBoardConnectionString"].ToString());
            SqlCommand cmd = new SqlCommand();
            DataSet ds = new DataSet();
            try
            {
                con.Open();
                cmd.Connection = con;

                SqlDataAdapter da = new SqlDataAdapter(cmd);
                da.Fill(ds);

            }
            catch (Exception ex)
            {
                throw ex;
            }

            finally
            {
                cmd.Dispose();
                con.Dispose();
            }

You just asked a very similar question here: http://stackoverflow.com/questions/9525307/does-sqltransaction-need-to-have-dispose-called — Cᴏʀʏ, Mar 01 '12 at 22:49

score 1 · Accepted Answer · answered Mar 01 '12 at 23:24

Actually, there is no need to worry about closing the connection when using the SqlDataAdapter.Fill(dataset) method. This method closes the connection after performing every Fill.

Also, there is no need to call SqlCommand.Dispose() since the command itself has no unmanaged resources to clean up. What you should be concerned about is if SqlConnection.Close() is called at some point. This is done after Fill.

the_joric · Answer 2 · 2012-03-01T22:55:00.120

0

The best practice is using using instead of try/finally :)

However in your case even using is not needed, because Fill() closes the connection:

        SqlConnection con = new SqlConnection(ConfigurationManager.AppSettings["MyNGConnectDashBoardConnectionString"].ToString())
        SqlDataAdapter da = new SqlDataAdapter("your sql is here", con);
        da.Fill(ds);

Also simple re-throwing exceptions makes no sense at all. If you need to log error, just use plain throw; as @Cory suggested.

edited Mar 01 '12 at 22:55

answered Mar 01 '12 at 22:48

the_joric

11,986
6
36
57

I know but I cant update 200 database calls and test them in the time I have so I have to work with what I have. – Mike Flynn Mar 01 '12 at 22:51
Added more details to my answer. – the_joric Mar 01 '12 at 22:54

score 0 · Answer 3 · answered Mar 01 '12 at 22:53

What you have is fine. It is always a good idea to dispose of objects that use unmanaged resources. However, if you get sick of always explicitly calling Dispose, the best practice is probably to use the using:

using (SqlConnection con = new SqlConnection(ConfigurationManager.AppSettings["MyNGConnectDashBoardConnectionString"].ToString()))
{
    using (SqlCommand cmd = new SqlCommand())
    {
        DataSet ds = new DataSet();
        try
        {
            con.Open();
            cmd.Connection = con;

            SqlDataAdapter da = new SqlDataAdapter(cmd);
            da.Fill(ds);

        }
        catch (Exception ex)
        {
            throw; // I changed this too!
        }
    }
}

Also, you almost always want to simply throw if you're going to "rethrow" an exception. You lose some of your stack trace if you throw ex;.

ADO.NET and Disposing without Using

3 Answers3