Is it OK to pass DataReaders to constructors?

Question

I'm maintaining some C# 2.0 code, and the programmer uses a pattern of reading a collection of business objects by opening a DataReader and then passing it to the object's constructor. I can't see anything obviously wrong with this, but it feels bad to me. Is this OK to do?

private static void GetObjects()
{
    List<MyObject> objects = new List<MyObject>();
    string sql = "Select ...";
    SqlConnection connection = GetConnection();
    SqlCommand command = new SqlCommand(sql, connection);
    connection.Open();
    SqlDataReader reader = command.ExecuteReader(CommandBehavior.CloseConnection);
    while (reader.Read())
        objects.Add(new MyObject(reader));
    reader.Close();
}

public MyObject(SqlDataReader reader)
{
    field0 = reader.GetString(0);
    field1 = reader.GetString(1);
    field2 = reader.GetString(2);
}

Answer 1

By passing the DataReader to the object's constructor, you establish a very tight coupling between the business object and your choice of persistence technology.

At the very least, this tight coupling will make reuse and testing difficult; at worst, it could result in the business objects knowing far too much about the database.

Resolving this isn't too difficult - you'd simply move object initialization out of the constructor and into a distinct factory class.

Answer 2

Passing a reader around makes me cringe, unless it's a non-public helper method to deal with copying one row. Definitely not to a constructor though.

Passing a reader connects your constructor (you didn't put MyObject in a class but you call new MyObject()) to your data storage and I presume your object isn't written to be such?

If it were me:

private static void GetObjects()
{
    List<MyObject> objects = new List<MyObject>();
    string sql = "Select ...";
    using (SqlConnection connection = GetConnection())
    {
        SqlCommand command = new SqlCommand(sql, connection);
        connection.Open();
        using(SqlDataReader reader = command.ExecuteReader(CommandBehavior.CloseConnection);)
        {
            while (reader.Read())
                objects.Add(_ReadRow(reader));
        }
    }
}

private static MyObject _ReadRow(SqlDataReader reader)
{
    MyObject o = new MyObject();
    o.field0 = reader.GetString(0);
    o.field1 = reader.GetString(1);
    o.field2 = reader.GetString(2);

    // Do other manipulation to object before returning

    return o;
}

class MyObject{}