¿Está bien pasar DataReaders a los constructores?

Question

Estoy manteniendo algún código C# 2.0, y el programador usa un patrón de lectura de una colección de objetos comerciales al abrir un DataReader y luego pasarlo al constructor del objeto. No puedo ver nada obviamente mal con esto, pero se siente mal para mí. ¿Esto esta bien?

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

Al pasar el DataReader al constructor del objeto, establece un acoplamiento muy ajustado entre el objeto comercial y su elección de tecnología de persistencia.

Por lo menos, este acoplamiento hermético dificultará la reutilización y la prueba; en el peor de los casos, podría hacer que los objetos de negocio supieran demasiado acerca de la base de datos.

Resolviendo esto no es demasiado difícil - simplemente movería la inicialización de objetos del constructor a una clase de fábrica distinta.

Answer 2

No lo haría de esta manera, pero no veo nada muy mal.

Answer 3

Pasarle un lector me da escalofríos, a menos que sea un método de ayuda no público para tratar de copiar una fila. Definitivamente no a un constructor sin embargo.

Pasar un lector conecta su constructor (no puso MyObject en una clase pero llama al new MyObject()) en su almacenamiento de datos y supongo que su objeto no está escrito para ser así.

Si se tratara de mí:

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{} 

Answer 4

(EDIT: Esta respuesta se centra únicamente en el "¿cuáles son las consecuencias a un nivel relativamente bajo" en lugar de las implicaciones generales de diseño se ve como otras respuestas han conseguido. los cubiertos, así que no voy a comentar :)

Bueno, definitivamente hay algo mal con el código que ha dado, ya que nada cierra la conexión, comando o lector. En concreto, la línea de asignación de conexión debe por lo general tener este aspecto:

using (SqlConnection connection = GetConnection()) 
{ 
    ... 
} 

bien puede pensar que esto es sólo puntillosa, y que es sólo código de muestra y la limpieza no es importante - pero la "propiedad" de recursos que necesitan limpiar es precisamente el problema al pasar un DataReader a un constructor.

Creo que está bien siempre que documente quién es el "dueño" del lector después. Por ejemplo, en Image.FromStream, la imagen posee la secuencia después y puede que no te importe que la cierres tú mismo (dependiendo del formato de la imagen y algunas otras cosas). Otras veces, sigue siendo su responsabilidad cerrar. Esto debe documentarse cuidadosamente, y si el tipo con el constructor toma posesión, debe implementar IDisposable para facilitar la limpieza y para que sea más obvio que se requiere limpieza.

En su caso, parece que el constructor es no haciéndose cargo del lector, que es una alternativa perfectamente fina (y más simple). Simplemente documente eso, y la persona que llama aún tendrá que cerrar al lector de manera apropiada.

Answer 5

Haría que la entidad pasara en un IDataReader, ya que esto ayuda a probar MyObject.

Answer 6

Llamaría a esto una "abstracción con fugas".

Me gusta usar objetos de persistencia en el ámbito más estrecho posible: adquirirlos, usarlos, limpiarlos. Si hubiera un objeto de persistencia bien definido, podría pedirle que asigne el resultado de la consulta en un objeto o colección, cierre el lector dentro del alcance del método y devuelva el objeto o colección a su cliente.

Answer 7

estoy totalmente de acuerdo con duffymo (1) (y Bevan)

Una idea que he estado masticando en mi mente es poco, si tiene que tener una dependencia, y por supuesto cuando me asigne una lector a un objeto que tiene que tener una dependencia, tal vez yo debería hacerla totalmente explícita y, de hecho, escribir un método de extensión para lograrlo, algo así como ...

//This Static extension class in the same namespace (but not necesarrily assembly) as my BO 

public static class DataReaderExtensions 
{ 
    public static List<MyObject> GetMyObjects(this DataReader reader) 
    { 

    } 
} 

esta manera, siempre y cuando estoy en el alcance de referencia de mi objeto comercial, mi lector de datos ahora tendrá un método adaptado a mis necesidades ... la idea probablemente deba desarrollarse más, pero creo que sería elegante.

Answer 8

Para capas de negocio y datos separados uso Rendimiento

public IEnumerable<IDataReader> getIDataReader(string sql) 
{ 

    using (SqlConnection conn = new SqlConnection("cadena de conexion")) 
    { 
     using (SqlCommand da = new SqlCommand(sql, conn)) 
     { 
      conn.Open(); 
      using (SqlDataReader dr = da.ExecuteReader) 
      { 
       if (dr.HasRows) 
       { 
        while (dr.Read) 
        { 
         yield return dr; 
        } 
       } 
      } 
      conn.Close(); 
     } 
    } 
} 

Answer 9

de pasar el lector de datos al constructor puede ser discutible, pero - por lo que yo sé - es un enfoque muy conocido. Pero quien lo usa, debe pasar una abstracción, es decir, no SqlDataReader, sino IDataReader. Sin embargo, IDataReader no es óptimo; ¡debería ser IDataRecord en su lugar, que comprende el resultado para el registro respectivo, en lugar de permitir iterar!