Disposing of SQL Connection

Question

I have a SQL class that connects to the DB and retreives a DataTable. I am aware that the SqlConnection must be disposed when finished. I know this can be done using a using block, but is it also acceptable to put the Dispose() call inside the destructor of this class?

Herre is my code:

public class SQLEng
{

    //Connection String Property
    //Must be set to establish a connection to the database
    public string ConnectionString{ get; set; }
    SqlConnection _Conn;

    //Overridden Constructor enforcing the Connection string to be set when created
    public SQLEng(string connectionString)
    {
        ConnectionString = connectionString;
        _Conn = new SqlConnection(connectionString);
    }

    //ensure the SqlConnection is disposed when destructing this object
    public ~SQLEng()
    {
        _Conn.Dispose();
    }

    //various other methods to get datatables etc...
}

Basically i wish to have a class variable SqlConnection, rather than instantiate the SqlConnection inside every method that accesses the DB. Is this sound practise?

The destructor is not guaranteed to be called. You should probably make `SQLEng` disposable itself and depend on users of _it_ to handle the disposal. — M.Babcock, Apr 12 '12 at 02:27
Thanks @m.babcock, i think i will take Yuck's advice and just open and close the connection when required. I would rather not have to call Dispose() every time i need DB access in other classes - simply trying to wrap it up in this class. — Simon, Apr 12 '12 at 02:33

score 8 · Accepted Answer · edited Apr 12 '12 at 02:37

Your design encourages hanging on to a (presumably open) SqlConnection for long periods of time. Best practice is to open a connection just before you need it and then release (close and dispose) it as soon as you are finished.

Yes, there is some overhead associated with creating new connections; connection pooling alleviates much of that processing time. Worse is keeping many connections alive on the server.

score 1 · Answer 2 · answered Apr 12 '12 at 02:37

Looking at the source for the Enterprise Library (from the MS Patterns & Practices team), the DAAB creates a connection as needed and disposes it as quickly as possible.

public virtual int ExecuteNonQuery(DbCommand command)
{
    using (var wrapper = GetOpenConnection())
    {
        PrepareCommand(command, wrapper.Connection);
        return DoExecuteNonQuery(command);
    }
}

protected DatabaseConnectionWrapper GetOpenConnection()
{
    DatabaseConnectionWrapper connection = TransactionScopeConnections.GetConnection(this);
    return connection ?? GetWrappedConnection();
}

So I would say that is a best practice. In most cases, all you are doing is returning the connection to the connection pool, so really the connection is not closed per se.

score 0 · Answer 3 · edited May 23 '17 at 10:30

If you wish to wrap the SQL Connection class, implement IDisposable and call the connection Dispose() from within your own Dispose() method. More info is here:

Properly disposing of a DbConnection

As to whether or not this is good practice - well, if all your doing is wrapped the SQL connection in another class, I'm not sure what you're achieving. All your methods will still need access to the instance of this class, in which case they could get access to the instance of the connection object by itself.

Disposing of SQL Connection

3 Answers3

Linked