MaiOM
MaiOM

Reputation: 936

Disposing SQL command and closing the connection

till now I always used a similar structure to get data from DB and fill a DataTable

public static DataTable GetByID(int testID)
        {
        DataTable table = new DataTable();
        string query = @"SELECT * FROM tbl_Test AS T WHERE T.testID = @testID";

        using (SqlConnection cn = new SqlConnection(Configuration.DefaultConnectionString))
        {
            SqlCommand cmd = new SqlCommand(query, cn);
            cmd.Parameters.Add("@testID", SqlDbType.Int).Value = testID;

            cn.Open();
            table.Load(cmd.ExecuteReader());
        }

        return table;
    }

Now I saw some warnings in the build analysis:

TestService.cs (37): CA2000 : Microsoft.Reliability : In method 'TestService.GetByID(int)', object 'table' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'table' before all references to it are out of scope.

TestService.cs (42): CA2000 : Microsoft.Reliability : In method 'TestService.GetByID(int)', call System.IDisposable.Dispose on object 'cmd' before all references to it are out of scope.

Should I change my code in

    public static DataTable GetByID(int testID)
    {
        DataTable table = new DataTable();
        string query = @"SELECT * FROM tbl_Test AS T WHERE T.testID = @testID";

        using (SqlConnection cn = new SqlConnection(Configuration.DefaultConnectionString))
        {
            using (SqlCommand cmd = new SqlCommand(query, cn))
            {
                cmd.Parameters.Add("@testID", SqlDbType.Int).Value = testID;

                cn.Open();
                table.Load(cmd.ExecuteReader());
            }
        }

        return table;
    }

What to do with DataTable object? Is it a good practice to place SqlCommand inside the using?

Thanks

Cheers

Upvotes: 11

Views: 5749

Answers (3)

lnu
lnu

Reputation: 1414

You should also do this:

using (SqlDataReader reader =
            cmd.ExecuteReader
                (CommandBehavior.CloseConnection))
        {
            table.Load(reader);
        }

when loading the table

Upvotes: 7

Matthew
Matthew

Reputation: 25763

To "fix" your issue with the DataTable, perhaps you could modify your function.

public static void GetByID(DataTable table, int testID)
{
    // bla bla bla
}


// calling the function
using(DataTable table = new DataTable())
{
    TestService.GetByID(table, 5);
}

Not saying this is the optimal solution, but it will solve the complaint.

Upvotes: 1

tafa
tafa

Reputation: 7326

  • The caller of this method should call the dispose of the DataTable returned when it is done using it.
  • Yes, it is a good practice to place SqlCommand inside using.

Upvotes: 3

Related Questions