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 -
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.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
}
}
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.