views:

229

answers:

3

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;
    }
+8  A: 

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

Otávio Décio
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
Very true. Thanks for the enlightenment.
simplyme
+3  A: 

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.

Neil Barnwell
A: 

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.

Eamon Nerbonne