Disposing SQL command and closing the connection

Question

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

Yes. Your code is correct now. Always dispose the class implementing IDisposable. But is SqlDataReader also disposable? — lnu
– lnu, Commented Dec 12, 2011 at 14:40
I believe the reader is disposable yes, and perhaps that is the issue WRT the table not being disposed as it holds a reader which has not been explicitly close, although I expect it will have been closed implicitly when the cmd is disposed. — Myles McDonnell
– Myles McDonnell, Commented Dec 12, 2011 at 14:45
You should do something like this: using (SqlDataReader reader = command.ExecuteReader (CommandBehavior.CloseConnection)) { dt.Load(reader); } — lnu
– lnu, Commented Dec 12, 2011 at 14:46
@Kangkan What do you mean dispose DataTable then return? I return that DataTable, I can't Dispose it there. — MaiOM
– MaiOM, Commented Dec 12, 2011 at 14:46

lnu · Accepted Answer · 2011-12-12 14:47:25Z

7

You should also do this:

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

when loading the table

answered Dec 12, 2011 at 14:47

lnu

1,4141 gold badge9 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

MaiOM Over a year ago

It is the first time that I see this approach. Why on MSDN nothing similar is even mentioned?

lnu Over a year ago

It's one option. MSDN shows only a sample(simple)case. And, yes you should dispose the datatable too.

MaiOM Over a year ago

Well as my are usually web applications, when should I call the disposal of the used DataTable? Lets say that on page load I associate that method call to the grid and Bind it. Where to call the dispose method? Thanks

lnu Over a year ago

If you use it with a datagrid or gridview, there is no good way to dispose it and you can leave it as it is. It's only if you're in another context(if datatable is not binded directly to a control) that you can dispose. Moreover it seems(google search) that there is no unmanaged resources in datatable though it implements IDisposable.

tafa · Accepted Answer · 2011-12-12 14:43:00Z

3

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.

answered Dec 12, 2011 at 14:43

tafa

7,3543 gold badges39 silver badges40 bronze badges

1 Comment

MaiOM Over a year ago

Well as my are usually web applications, when should I call the disposal of the used DataTable? Lets say that on page load I associate that method call to the grid and Bind it. Where to call the dispose method? Thanks

Matthew · Accepted Answer · 2011-12-12 14:46:57Z

1

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.

answered Dec 12, 2011 at 14:46

Matthew

26k9 gold badges86 silver badges118 bronze badges

Collectives™ on Stack Overflow

Disposing SQL command and closing the connection

3 Answers 3

4 Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related