Return SqlDataReader with closed connection

Question

I have created a Sql helper class which handles most of my needs.

Amongst those, I have a function which executes a SQL statement and returns a SqlDataReader as follows:

public static SqlDataReader ExecuteCommand(string cmdText, bool useParameters)
{
    using (var sqlConnection = new SqlConnection(_connectionString.ConnectionString))
    {
        sqlConnection.Open();
        using (var sqlCommand = new SqlCommand(cmdText, sqlConnection))
        {
            if (useParameters && SqlParameterCollection.Count > 0)
            {
                sqlCommand.Parameters.AddRange(SqlParameterCollection.ToArray());
            }

            using (var sqlDataReader = sqlCommand.ExecuteReader())
            {
                return sqlDataReader;
            }
        }
    }
}

The problem with this is, obviously, the fact that it returns a sqlDataReader which requires an open connection and the fact that the connection is closed.

I have looked at returning a SqlDataAdapter instead, but after reading the following thread SqlDataAdapter vs SqlDataReader it doesn't sound like such a good idea as a general function to be used in every single scenario when you have absolutely no idea of the amount of data it is supposed to load.

So... What's a good alternative?

The only thing I can think of off the top of my head is to cycle through the rows in the SqlDataReader and do a yield return IEnumerable<IDataRecord>.

Is there any better way of achieving this or is this pretty much it?

Answer 1

You could use CommandBehavior.CloseConnection, which offloads closure to the caller - but this then makes your code dependent on the caller using the reader correctly.

Personally, I would try to minimize this type of dependency - and move the materialization code as close to the DB code as possible. In reality, there aren't that many cases when the caller needs a raw reader - or at least, there shouldn't be. I would strongly advise using tools like "dapper" or ORMs, so that you can just do something like:

return connection.Query<T>(cmdText, args).ToList();

which then doesn't leave many places for the caller to make a mess of things.

Answer 2

You can create a wrapper for your connection and reader, which implements System.IDisposable. The wrapper becomes responsible for closing the connection:

public class DbReaderWrapper : IDbReaderWrapper
    {
        public DbDataReader Reader { get; private set; }
        private readonly DbConnection _connection;
        private bool _disposed = false;

        public DbReaderWrapper(DbConnection connection, DbDataReader reader)
        {
            Reader = reader;
            _connection = connection;
        }

        ~DbReaderWrapper()
        {
            Dispose(false);
        }

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

        protected virtual void Dispose(bool disposing)
        {
            if(_disposed)
            {
                return;
            }

            if(disposing)
            {
                _connection.Dispose();
            }

            _disposed = true;
        }
    }

Your ExecuteCommand() function would look something like this:

public static SqlDataReader ExecuteCommand(string cmdText, bool useParameters)
{
    var sqlConnection = new SqlConnection(_connectionString.ConnectionString)
    sqlConnection.Open();
    var sqlCommand = new SqlCommand(cmdText, sqlConnection)

    if (useParameters && SqlParameterCollection.Count > 0)
    {
        sqlCommand.Parameters.AddRange(SqlParameterCollection.ToArray());
    }

    var sqlDataReader = sqlCommand.ExecuteReader()
    return new DbReaderWrapper(sqlConnection, sqlDataReader);
}

And you can call this method with the using statement:

using var readerWrapper = sqlHelper.ExecuteCommand(query, true) {
    // Do something with the reader
}