Lifecycle of Db connection objects

Question

Details - I have a SQL Database helper in C# with a method (like below) which establishes a connection

    public void EstablishConnection()
    {

        oConnection = oFactory.CreateConnection();

        if (oConnection.State == ConnectionState.Closed)
        {
            oConnection.ConnectionString = StringConnection;
            oConnection.Open();
            oConnectionState = ConnectionState.Open;
        }
    }

For any calls made to db methods like (ExecuteNonQuery etc) this is how a typical method looks like

  public DataSet DataAdapter(CommandType cmdType, string cmdText, Parameters[] cmdParms)
    {

        DbDataAdapter dda = null;
        try
        {
            EstablishFactoryConnection();
            dda = oFactory.CreateDataAdapter();
            PrepareCommand(false, cmdType, cmdText, cmdParms);

            dda.SelectCommand = oCommand;
            DataSet ds = new DataSet();
            dda.Fill(ds);
            return ds;
        }
        catch (Exception ex)
        {
            throw ex;
        }
        finally
        {
            if (null != oCommand)
                oCommand.Dispose();
            CloseFactoryConnection();
        }
    }

And the CloseFactoryConnection(); method is like below

 public void CloseFactoryConnection()
    {
        //check for an open connection            
        try
        {
            if (oConnection.State == ConnectionState.Open)
            {
                oConnection.Close();
                oConnectionState = ConnectionState.Closed;
            }
        }
        catch (DbException oDbErr)
        {
            //catch any SQL server data provider generated error messag
            throw new Exception(oDbErr.Message);
        }
        catch (System.NullReferenceException oNullErr)
        {
            throw new Exception(oNullErr.Message);
        }
        finally
        {
            if (null != oConnection)
                oConnection.Dispose();
        }
    }

Questions - I have this class (DBHelper) being used in all my repositories where the IOC container constructor injects this. The lifestyle of these instances is HttpContextLifecycle (I am using Structuremap which says this lifestyle means that the A single instance will be created for each HttpContext. Caches the instances in the HttpContext.Items collection.)

Is this the right TYPE of lifestyle for the db connection class i am using?

Should the class creating the connection/ opening it/ closing it be instantiated as a singleton? Or Per request?

What is the generally accepted lifestyle for these db connection establishing classes? I am using ado.net and there is no ORM.

Thanks

score 2 · Answer 1 · edited May 23 '17 at 12:20

Because ADO.NET already uses connection pooling, there is no need to cache/reuse DbConnection instances. Given this, your class doesn't appear to add anything useful, as opposed to simply writing out the code -- the only thing you're abstracting away is the connection string, and that's adequately handled by making it configurable (ConnectionStrings["DatabaseName"]).

Aside from the lack of added value, your class has other problems. Catching NullReferenceException is a big no-no -- a NullReferenceException indicates a serious bug in your code and should be fixed by investigating the crash, not by wrapping it as another exception (and when you are wrapping, do not use Exception, because it's unspecific and clients cannot catch it without catching everything, which is another bad thing to do). Better yet, of course, it should be prevented by appropriately checking for null (see What is a NullReferenceException, and how do I fix it?)

To see why your class is probably not useful, consider that CloseFactoryConnection could be simplified to the following without any loss in functionality:

public void CloseFactoryConnection() {
    using (oConnection) { }
}

So to give a broad answer to your question: if you're not using an ORM, you probably don't want a class for establishing database connections at all, beyond the ones you've already got. If you want to use dependency injection for getting data, don't use it at this level, but at the object level -- you want to be able to abstract away from using the database at all, not merely from how a DataSet is created.

Thanks for your reply. I take this on board, however this is some legacy code that i cannot remove (as its being used through out). What do you think of the lifestyle of the instantiated object if i WERE to use this way of dealing with db connections etc? Do you think per request is appropriate? — nesh_s, Oct 20 '14 at 13:27
If this class is a given and all you have to decide is when to instantiate it, then for your own sanity, make it per request. This is because a class like this represent shared state, and you have the least chance for bugs if the state is in fact not shared at all. Any performance hit should be negligible because, as I said, it doesn't actually seem to do much. — Jeroen Mostert, Oct 20 '14 at 13:43

Lifecycle of Db connection objects

1 Answers1