Search code examples
c#idisposablesqlcommandsqlclient

Is it correct to not dispose of SqlCommand like the example in Microsoft documentation?


The Microsoft Documentation for the SqlCommand class gives the following example code which does not dispose of the SqlCommand.

SqlCommand inherits from DbCommand which implements IDisposable.

Common knowledge would suggest it's advisable to dispose of the SqlCommand, however, the example does not.

Is the example providing a best practice?

private static void ReadOrderData(string connectionString)
{
    string queryString =
        "SELECT OrderID, CustomerID FROM dbo.Orders;";
    using (SqlConnection connection = new SqlConnection(
               connectionString))
    {
        SqlCommand command = new SqlCommand(
            queryString, connection);
        connection.Open();
        using(SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                Console.WriteLine(String.Format("{0}, {1}",
                    reader[0], reader[1]));
            }
        }
    }
}

Solution

  • See my comments in your code first

    private static void ReadOrderData(string connectionString)
    {
        string queryString = "SELECT OrderID, CustomerID FROM dbo.Orders;";
        using (SqlConnection connection = new SqlConnection(connectionString))
        {
            SqlCommand command = new SqlCommand(queryString, connection);
            connection.Open();
            using(SqlDataReader reader = command.ExecuteReader())
            {
                while (reader.Read())
                {
                    Console.WriteLine(String.Format("{0}, {1}", reader[0], reader[1]));
                }
            } // -- disposes reader and closes connection
        } // -- disposes connection and closes it 
    }
    

    command was declared in the scope of connection using, i.e. try/catch/finally Command is not necessarily holding any resources outside of connection. And you disposing connection, which already closed when reader is disposed. (see CommandBerhavior). So, in the end, command is the object that holds references to disposed resources.

    To know better, use some reflection tool and look inside.

    protected override void Dispose(bool disposing)
    {
        if (disposing)
            this._cachedMetaData = (_SqlMetaDataSet) null;
        base.Dispose(disposing);
    }
    

    So, what is this metadata? Probably some Parameter info. But if you don't add parameters then you might have nothing to dispose. Microsoft knows this and found unnecessary to include dispose on command. This is to answer, why Microsoft did not include using for command.

    Look at the constructor

    public SqlCommand()
    {
        GC.SuppressFinalize((object) this);
    }
    

    Looks like there is nothing to dispose in the first place. And all those "cached items" will be garbage collected normally. I did not find there any IO or other unmanaged resources anywhere there.

    One thing however, to remember, is that implementation might change from version to version and you don't want to change the code. So, add another layer to command

    private static void ReadOrderData(string connectionString)
    {
        string queryString = "SELECT OrderID, CustomerID FROM dbo.Orders;";
    
        using (SqlConnection connection = new SqlConnection(connectionString))
        using (SqlCommand command = new SqlCommand(queryString, connection))
        { 
           . . . . . . 
        }   
    }
    

    This will potentially save some hustle.

    Fun fact: When working with MySql provider which implements same interfaces and base classes as SqlClient, there was a bug (still is probably) in the MySqlCommand.Dispose which closed the reader always. And this same code with using would work with SqlClient and Oracle ODP but not MySql. I had to wrap MySqlCommand and use my custom class (think of Decorator pattern) in the provider-agnostic code to overwrite the behavior.