Creating an IDisposable class in c# which cleans up an SqlConnection when finished with

Question

In an answer to a previous question somebody recommended:

have the SqlConnection a member variable of your class, but make the class IDisposable and dispose of the SqlConnection when the class is disposed

I have put together an implementation of this suggestion (below) but wanted to check that this implementation is correct (obviously it doesn't currently do anything except open the connection but the idea is that there would be methods in there which would use the connection and which would be able to rely on it existing and being open).

public class DatabaseRecord : IDisposable
{
    protected SqlConnection connection;

    public DatabaseRecord()
    {
        connection = new SqlConnection("ConnectionString");
        connection.Open();
    }

    // IDisposable implementation

    private bool disposed;
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }


    private void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                connection.Dispose();
            }
            disposed = true;
        }

    }

    // Destructor
    ~DatabaseRecord()
    {
        Dispose(false);
    }
}

Will this work? Will classes which use instances of DatabaseRecord need to do anything special or will the Dispose automatically be called once the instances are no longer used/ referenced? Is this more efficient/ better than using using (var connection = new SqlConnection("...")) { } in each separate method body where the connection is needed?

Answer 1

SqlConnection is a managed resource, and should be disposed within the if (disposing) block. Classes using your class should dispose it, ideally witrh a using block. Whether this is better than individual using blocks for SqlConnections will depend on the other methods of this class and how they are used.

Answer 2

All the advice I've seen says that DbConnections should be kept around for the minimum time, so the format I'd prefer to see in code I'm reviewing is

using (var connection = new SqlConnection("...")) { ... }

Answer 3

connection.Dispose() should be moved to if (disposing) { ... } block. The calling of Close() is not required because Dispose() will close the connection when connection is open.

Answer 4

This will work and it will be more efficient that multiple using statements. The code that uses the DatabaseRecord class can do so inside a using statement to have it automatically cleaned up there when it leaves the loop.

One recommendation however, would be in the Dispose method to check the state of the Connection object and close it if it is still open before calling dispose.