È corretto passare i DataReader ai costruttori?

Question

Sto mantenendo un codice C# 2.0 e il programmatore utilizza uno schema di lettura di una raccolta di oggetti business aprendo un DataReader e quindi passandolo al costruttore dell'oggetto. Non riesco a vedere nulla di evidentemente sbagliato in questo, ma mi fa male. Va bene questo?

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

Passando il DataReader al costruttore dell'oggetto, si stabilisce un accoppiamento molto stretto tra l'oggetto business e la scelta della tecnologia di persistenza.

Per lo meno, questo accoppiamento stretto renderà difficile il riutilizzo e il collaudo; nel peggiore dei casi, potrebbe portare gli oggetti di business a conoscere fin troppo il database.

La risoluzione di questo non è troppo difficile: devi semplicemente spostare l'inizializzazione dell'oggetto dal costruttore e in una classe factory distinta.

Answer 2

Non vorrei farlo in questo modo, ma non vedo nulla di gravemente sbagliato.

Answer 3

Passare un lettore in giro mi fa rabbrividire, a meno che non si tratti di un metodo di supporto non pubblico per gestire la copia di una riga. Sicuramente non per un costruttore però.

Passare un lettore collega il costruttore (non hai messo MyObject in una classe ma chiami new MyObject()) nella tua memoria dati e presumo che il tuo oggetto non sia scritto per essere tale?

Se fossi stato io:

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: Questa risposta si concentra esclusivamente sul "quali sono le implicazioni a un livello relativamente basso", piuttosto che le implicazioni complessive di progettazione E si presenta come altre risposte hanno ottenuto. quelli coperti, quindi non voglio commentare :)

Beh, c'è sicuramente qualcosa di sbagliato con il codice che hai dato, in quanto nulla chiude la connessione, comando o lettore. In particolare, la linea di assegnazione dei collegamenti di solito dovrebbe assomigliare a questo:

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

si può ben pensare che questo è solo pignoleria, e che è solo il codice di esempio e il clean-up non è importante - ma la "proprietà" di risorse che richiedono pulizia è precisamente il problema con il passaggio di un DataReader a un costruttore.

Penso che sia a posto finchè si documenta chi "possiede" il lettore in seguito. Ad esempio, in Image.FromStream, l'immagine possiede lo stream in seguito e potrebbe non essere gentile a chiederlo da solo (a seconda del formato dell'immagine e di alcune altre cose). Altre volte è ancora tua responsabilità chiudere. Questo deve essere documentato molto attentamente e se il tipo con il costruttore diventa proprietario, è necessario implementare IDisposable per semplificare la pulizia e per rendere più evidente la necessità di eseguire la pulizia.

Nel tuo caso, sembra che il costruttore sia non che si appropria del lettore, che è un'alternativa perfettamente valida (e più semplice). Basta documentarlo e il chiamante dovrà comunque chiudere il lettore in modo appropriato.

Answer 5

Vorrei rendere l'entità passata in un IDataReader in quanto questo aiuta a testare il MyObject.

Answer 6

Chiamerei "astrazione che perde".

Mi piace utilizzare gli oggetti di persistenza nel più stretto ambito possibile: acquisirli, usarli, pulirli. Se esisteva un oggetto di persistenza ben definito, è possibile chiedergli di associare il risultato della query a un oggetto o una raccolta, chiudere il lettore nell'ambito del metodo e restituire l'oggetto o la raccolta al client.

Answer 7

Sono totalmente d'accordo con duffymo (1) (e Bevan)

Un'idea che ho masticare più volte nella mia mente di recente è, se io avere una dipendenza, e, naturalmente, quando ho mappare una lettore a un oggetto devo avere una dipendenza, forse dovrebbe rendere completamente esplicito e di fatto scrivere un metodo di estensione per raggiungerlo, qualcosa di simile ...

//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) 
    { 

    } 
} 

in questo modo, fino a quando io sono in l'ambito di riferimento del mio oggetto business, il mio datareader ora avrà un metodo su misura per le mie esigenze ... l'idea probabilmente deve essere arricchita di più, ma penso che sarebbe elegante.

Answer 8

Per strati di business e dati separati all'uso Resa

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

passa al lettore i dati alla funzione di costruzione può essere discutibile, ma - per quanto ne so - è un approccio ben noto. Ma chiunque lo usi, dovrebbe passare un'astrazione, cioè non SqlDataReader ma IDataReader. Tuttavia IDataReader non è ottimale; dovrebbe invece essere IDataRecord, che comprende il risultato per il rispettivo record, invece di consentire l'iterazione !!