How to manage SqlDataReaders in a data access layer?

Question

I am trying to get a better handle on decoupling my code, code reuse, etc.

I'm tired of typing the below every time I want to read some rows:

using(SqlConnection conn = new SqlConnection(myConnString))
{
  using(SqlCommand cmd = new SqlCommand(cmdTxt, conn))
  {
    conn.Open();
    using(SqlDataReader rdr = cmd.ExecuteReader())
    {
       while(rdr.Read())
       {
          /* do something with rows */
       }
     }
   }
}

I understand there is LINQ to SQL (I don't like it), and the Entity Framework (still a baby). I have no problems having to type my queries out, I just don't want to have to type the command contruction, row iterator, etc each time.

I looked around and found something that I thought would work for me, and tried to implement it to make things easier for me. As you can see in the comment, I get an error that the SqlDataReader is closed. I'm guessing it's probably because of the using statement int the DataFactory.ExecuteReader() method. When the reader is returned, the dispose method is called on my SqlConnection and SqlCommand variables. Am I right there? If so, how should one manage the connection and command variables?

Edit: I updated my code example to better reflect what I am doing.

public class DataFactory
{
    public DataFactory()
    {}

    public DataFactory(string connectionString)
    {
       _connectionString = connectionString;
    }

    protected _connectionString = "Data Source=Localhost, etc, etc";
    private string ConnectionString
    {
        get{return _connectionString;}
    }

    public SqlConnection GetSqlConnection()
    {
        return new SqlConnection(ConnectionString);
    }

    public SqlDataReader ExecuteReader(string cmdTxt)
    {
        using(SqlConnection conn = new SqlConnection(ConnectionString))
        {
           using(SqlCommand cmd = new SqlCommand(cmdTxt, conn))
           {
                conn.Open();
                return cmd.ExecuteReader();
           }
        }
    }
}

public IRepository<T>
{
    T GetById(int id);
}

public MyTypeRepository: IRepository<MyType>
{
   private static DataFactory _df = new DataFactory();

   public MyType GetById(int id)
   {
        string cmdTxt = String.Format("SELECT Name FROM MyTable WHERE ID = {0}", id);

        using(SqlDataReader rdr = _df.ExecuteReader(cmdTxt))
        {
            if(rdr.Read()) /* I get an error that the reader is already closed here */
            {
                return new MyType(
                    Convert.ToInt32(rdr["Id"]),
                    rdr["Name"]);
            }
            else
            {
                return null;
            }
        }        
    }
}




public class MyType
{
    public MyType(int id, string name)
    {
      _id = id;
      _name = name;
    }

    private string _name;
    public string Name
    {
       get{return _name;}
    }

    private int _id;
    public int Id
    {
        get{return _id;}
    }

    public override void ToString()
    {
        return string.Format("Name: {0}, Id: {1}", Name, Id);
    }
}

public class Program
{
    private static MyTypeRepository _mtRepo = new MyTypeRepository();

    static void Main()
    {
        MyType myType = _mtRepo.GetById(1);

        Console.WriteLine(myType.ToString());
    }
}

I also would like to know if what I'm doing makes any sense, or, if not, how to achieve something similar so that I don't have to type the connection creation, etc so often.

Answer 1

I would say it's not really decoupling enough - basically any module you have with "using System.Data.SqlClient" is coupled to your database. The whole point of a DAL is that the application is coupled to the DAL and the DAL is coupled to the database.

Answer 2

What I do is I create an XML file with my queries and use an XSLT transformation to generate my DAL code CS files. You can go as fancy as you like, declare parameters in the XML and generate methods with appropriate signatures in the XSLT etc etc. I have a blog entry that covers, for a related topic, how to integrate the XSLT transformation into your Visual Studio project. Now one may argue that using a typed dataset is the same thing and is a free lunch, but in my case I use asynchronous DAL based on BeginExecute/EndExecute. None of the VS tools gets this approach right so I basically had to build my own.

Answer 3

It's very important that you close and/or dispose your data reader after using it then everyone who wants to use your DataFactory should remember to do that.I think it's a good idea to return a DataTable instead of SqlDataReader so that your DataFactory is not dependent to SqlDataReader.

I mean :

public DataTable ExecuteReader(string cmdTxt)
    {
        using(SqlConnection conn = new SqlConnection(ConnectionString))
        {
           using(SqlCommand cmd = new SqlCommand(cmdTxt, conn))
           {
                conn.Open();
                using(SqlDataReader reader=cmd.ExecuteReader())
                {
                    DataTable dt=new DataTable();
                    dt.Load(reader);
                    return dt;
                }

           }
        }
    }

EDIT: Good point.I don't like data tables either ( We use NHibernate so I actually don't use data tables in our applications) So if you'd like to map a data reader to your own objects maybe you can have a data mapper that maps data reader to your own objects I mean:

public T[] ExecuteReader<T>(string cmdTxt)
    {
        using(SqlConnection conn = new SqlConnection(ConnectionString))
        {
           using(SqlCommand cmd = new SqlCommand(cmdTxt, conn))
           {
                conn.Open();
                using(SqlDataReader reader=cmd.ExecuteReader())
                {
                    var result=new List<T>();
                    while(reader.Read())
                         result.Add(ObjectMapper.MapReader<T>(reader));

                    return result.ToArray();
                }

       }
    }
}

Answer 4

Your method ExecuteReader will close the connection before returning the Reader. Instead it should be implemented something like:

public IDataReader ExecuteReader(string cmdTxt)    
{        
    SqlConnection conn = new SqlConnection(...);
    try
    {
        SqlCommand cmd = new SqlCommand(cmdTxt, conn);
        conn.Open();                
        return cmd.ExecuteReader(CommandBehavior.CloseConnection);           
    }
    catch
    {
        conn.Close();
        throw;
    }
}

Callers of the ExecuteReader method will need to dispose the IDataReader:

using(IDataReader reader = ExecuteReader(commandText))
{
    ...
} // reader will be disposed here and will close the connection.

Note that the above does not call Dispose on the SqlCommand object. In my experience and from looking at SqlCommand with Reflector it's not necessary as long as the SqlConnection is disposed. But I believe the following will work if you do want to dispose it:

public IDataReader ExecuteReader(string cmdTxt)    
{        
    SqlConnection conn = new SqlConnection(...);
    SqlCommand cmd = null;
    try
    {
        cmd = new SqlCommand(cmdTxt, conn);
        conn.Open();                
        IDataReader reader = 
            cmd.ExecuteReader(CommandBehavior.CloseConnection);           
        cmd.Dispose();
        return reader;
    }
    catch
    {
        if (cmd != null) cmd.Dispose();
        conn.Close();
        throw;
    }
}

Answer 5

You need something like this http://aspalliance.com/526