Keeping the SqlDataReader open across an n-tiered layer

Question

I have a Database class that abstracts the ExecuteNonQuery() and ExecuteReader() of SqlCommand. Due to wrapping the Sqlconnection and SqlCommand around using blocks, the SqlDataReader gets closed after the CustomExecuteReader() is called, therefore I can't read the SqlReaderResultSet at the business level layer. Code below. Thanks guys for the feedback.

public static SqlDataReader SqlReaderResultSet { get; set; }    

public static SqlDataReader CustomExecuteReader(string storedProc)
    {
        using (var conn = new SqlConnection(ConnectionString))
        {
            var cmd = new SqlCommand(storedProc, conn) {CommandType = CommandType.StoredProcedure};                

            try
            {
                conn.Open();
                SqlReaderResultSet = cmd.ExecuteReader();
            }
            catch (InvalidOperationException)
            {
                if (conn.State.Equals(ConnectionState.Closed))
                    conn.Open();
            }
            finally
            {                    
                conn.Close();
            }

        }
        return SqlReaderResultSet;
    }

Remember that data readers keep live connections to the database. If the result set is small enough, use a dataset instead. Otherwise this approach screams connection leaks — Brian Rudolph, Sep 16 '09 at 13:42

score 11 · Accepted Answer · answered Sep 16 '09 at 11:33

11

"I can't read the SqlReaderResultSet at the business level layer" - and you shouldn't. Data should be passed using data transfer objects, never through a low level data access structure.

answered Sep 16 '09 at 11:33

Otávio Décio

73,752
17
161
228

While I agree with the idea, adding data transfer objects can mean including a lot of boilerplate code which does almost nothing. At that point, it's reasonable to wonder whether all that meaningless code serves a purpose. An SqlDataReader isn't a good thing to be passing around as a datastructure, but discerning those wholesome "high level data structures" from "low level data access structures" isn't always black and white. – Eamon Nerbonne Sep 16 '09 at 12:01
Very true. Thanks for the enlightenment. – simplyme Sep 24 '09 at 16:57

score 4 · Answer 2 · answered Sep 16 '09 at 11:35

4

I recommend changing your approach so that the method you describe above iterates the records in the datareader, and creates a list of objects. That list of objects is what should be returned and worked on.

answered Sep 16 '09 at 11:35

Neil Barnwell

41,080
29
148
220

score 0 · Answer 3 · answered Sep 16 '09 at 11:55

Iterator Blocks can be a way around this. It is legal and generally safe to do the following:

IEnumerable<MyFancyData> ResultSet {
    get {
        using(DbConnection conn = ...) 
        using(DbCommand cmd = ...) {
            conn.Open();

            using(DbDataReader reader = cmd.ExecuteReader()) {
                while(reader.Read()) {
                    yield return new MyFancyData(reader[0], reader[42] ...);
                }
            }
        }
    }
}

Each time you enumerate the ResultSet property, the connection will be constructed again - and Disposed of afterwards (foreach and other IEnumerator<> consumers will appropriately call the Dispose() method of the generator, allowing the using block to do its thing).

This approach retains the lazy as-you-need it evaluation of the items from the data reader (which can be relevant when your data set becomes large), which still cleaning abstracting away sql-level details from the public API.

Keeping the SqlDataReader open across an n-tiered layer

3 Answers3