Disposing SqlConnection from a base DAL class when calling ExecuteReader

Question

I've been assigned on a project where the DAL consists of a base class with functions to return IDataReader, an Object (int, string and the like), or a DataSet. An ExecuteNonQuery function also exists. This DAL only accesses USPs (SQL Server), and is using MS's SqlHelper to execute the queries. Here's two sample functions from the base:

    protected IDataReader ExecuteReader(string storedProcedure, params object[] parameterValues)
    {
        SqlConnection HConnection = new SqlConnection(myConnString);
        IDataReader ret = null;
        try
        {
            ret = SqlHelper.ExecuteReader(HConnection, storedProcedure, parameterValues);
        }
        catch (Exception ex)
        {
            HanldeError(ex, storedProcedure, parameterValues);
        }
        return ret;
    }

    protected object ExecuteScalar(string storedProcedure, params object[] parameterValues)
    {
        using (SqlConnection HConnection = new SqlConnection(myConnString))
        {
            object ret = null;
            try
            {
                ret = SqlHelper.ExecuteScalar(HConnection, storedProcedure, parameterValues);
            }
            catch (Exception ex)
            {
                HanldeError(ex, storedProcedure, parameterValues);
            }
            return ret;
        }
    }

Other classes derive from this base class, creating task-specific DAL classes, for instance:

public class Orders : BaseDal {
    public IDataReader GetOrdersList(int clientId, int agentId)
    {
        return ExecuteReader("usp_Orders_GetOrdersList", clientId, agentId);
    }
    ...
}

Then there are BLL classes that call the DAL functions, and fill objects (for example an Order object) with the data based on the data read from the IDataReader object:

    public Order[] GetOrdersList(int ClientIDX, int AgentIDX)
    {
        List<Order> ret = null;
        using (IDataReader dr = objDAL.GetOrdersList(ClientIDX, AgentIDX))
        {
            if (dr != null)
            {
                ret = new List<Order>();
                while (dr.Read())
                {
                    ret.Add(xReadOrder(dr, 0));
                }
            }
        }
        return ret.ToArray();
    }

My question is this - if you'll look at the code taken from BaseDal, you'll notice only ExecuteScalar actually terminates the SqlConnection object (the using statement) - this is the case with all my functions there. With ExecuteReader I cannot do that, as I'm returning an open SqlDataReader object and closing the connection will invalidate the reader. I have all the code getting and using an IDataReader from the DAL use the using statement, but is the SqlConnection object being disposed as well, or is it being GC'ed at a later stage, hurting the connection pooling by not freeing it up sooner? If so, how can this be treated?

Also, is there a better approach to creating a DAL than the one described above? I'm less concerned about data store agnostic DAL, we only need a solid and easy to maintain one, one that can take many concurrent connections from many threads.

Thanks in advance for any help on this.

Answer 1

SqlDataReader will close the connection if you specify CommandBehavior.CloseConnection when you execute the command. However, it feels a bit ugly anyway.

Instead, pass a SqlConnection into the method and use that. Then the caller has control over when that's disposed. Alternatively, take an Action<SqlDataReader> to execute with the open reader, and make the method close both the reader and the connection, after executing the action. That action would have to then do everything it needed to with the reader, of course.