Search code examples
c#idisposable

Does IDisposable always have to be disposed?


I have a class that contains a protected IDbConnection property. Within this class I have a function that will return an open IDbConnection. As a result, I thought the class should also implement the IDisposable class to ensure the IDbConnection gets disposed of properly.

This is what my class looks like -

public class MyConnection : IDisposable
{
    protected IDbConnection _connection;
    protected string _connectionString;

    public IDbConnection OpenConnection()
    {
        _connection = new SqlConnection(_connectionString);
        _connection.Open();
        return _connection;
    }

    public void Dispose()
    {
        if(_connection != null)
        {
            _connection.Dispose();
        }
    }
}

The way I use it is -

MyConnection mc = new MyConnection();

using(var cnn = mc.OpenConnection())
{
    // do stuff with cnn
}

That should handle the disposal of the connection when it is finished. Couple of questions on this -

  1. Is the IDisposable inheritance necessary for MyConnection? My thinking is that it is just best practice since you could potentially have an open connection out of the class control.
  2. If the IDisposable is required, should I instead be using it like this -

Disposing of both

using(var mc = new MyConnection())
{
    using(var cnn = mc.OpenConnection())
    {
        // do stuff
    }
}

Solution

  • As noted in the comments, what you have there should rather be named MyConnectionFactory, because that's what it does.

    Since you're returning the connection, that is what needs to be disposed, not the factory itself. So your use case

    MyConnectionFactory mc = new MyConnectionFactory();
    
    using(var cnn = mc.OpenConnection())
    {
        // do stuff with cnn
    }
    

    is the right approach. Disposing mc doesn't do anything useful here, because it disposes a member that is actually already disposed. You should even remove the member _connection from the class, because it doesn't really help you to keep it, as the caller cleans it up. That is, unless you want to keep the connection open, but then the callers must not dispose it.