Czy można przekazać obiekt DataReaders do konstruktorów?

Question

Zachowuję trochę kodu C# 2.0, a programista używa wzorca do czytania zbioru obiektów biznesowych, otwierając obiekt DataReader, a następnie przekazując go do konstruktora obiektu. Nie widzę w tym nic złego, ale wydaje mi się to złe. Czy to jest w porządku?

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

Przekazując obiekt DataReader do konstruktora obiektu, tworzy się bardzo ścisłe połączenie między obiektem biznesowym a wybraną technologią utrwalania.

Przynajmniej te ścisłe sprzężenie utrudni ponowne użycie i testowanie; w najgorszym przypadku może to spowodować, że obiekty biznesowe będą wiedziały o zbyt wiele na temat bazy danych.

Rozwiązanie tego nie jest zbyt trudne - wystarczy przenieść inicjalizację obiektu z konstruktora do odrębnej klasy fabrycznej.

Answer 2

Nie chciałbym zrobić to w ten sposób, ale nie widzę niczego Majorly złego.

Answer 3

Przekazywanie czytelnikowi sprawia, że ​​wzdragam się, chyba że jest to niepubliczna metoda pomocnicza do radzenia sobie z kopiowaniem jednego wiersza. Zdecydowanie nie do konstruktora.

Przekazanie czytnika łączy twojego konstruktora (nie umieściłeś MyObject w klasie, ale wywołujesz new MyObject()) do przechowywania danych i zakładam, że twój obiekt nie jest tak napisany?

Gdyby mnie:

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: Ta odpowiedź skupia się wyłącznie na „jakie są implikacje na stosunkowo niskim poziomie”, a nie ogólnych skutków projektowych wygląda jak inne odpowiedzi mam. te objęte, więc nie będę komentować :)

Coś jest nie tak z kodem, który podałeś, ponieważ nic nie zamyka połączenia, polecenia lub czytnika. W szczególności, linia zadanie połączenie powinno zazwyczaj wygląda tak:

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

Być może dobrze, że to jest właśnie nit-picking, i że to tylko przykładowy kod i czyste-up ma znaczenia - ale „własności” Zasoby, które potrzebują do czyszczenia, to właśnie problem z przekazaniem do konstruktora DataReader.

Myślę, że to w porządku, o ile dokumentujesz, kto "jest właścicielem" czytnika. Na przykład, w Image.FromStream, obraz jest właścicielem strumienia i może nie być przyjazny dla Ciebie zamykając go sam (w zależności od formatu obrazu i kilku innych rzeczy). Innym razem to twój obowiązek zamknąć. Musi to być bardzo starannie udokumentowane, a jeśli typ z konstruktorem stanie się własnością, powinien wdrożyć , aby oczyścić je łatwiejsze: i, aby było bardziej oczywiste, że wymagane jest czyszczenie.

W twoim przypadku wygląda na to, że konstruktorem jest , a nie przejmowanie na własność czytnika, co jest doskonałą (i prostszą) alternatywą. Po prostu dokumentuj to, a osoba dzwoniąca będzie nadal musiała odpowiednio zamknąć czytnik.

Answer 5

Sprawię, że podmiot zostanie przekazany w IDataReader, ponieważ pomoże to w testowaniu MyObject.

Answer 6

Nazwałbym to "nieszczelną abstrakcją".

Lubię używać obiektów uporczywości w możliwie najwęższym zakresie: je zdobywać, używać ich, czyścić. Jeśli istnieje dobrze zdefiniowany obiekt utrwalania, możesz poprosić go o odwzorowanie wyniku zapytania na obiekt lub kolekcję, zamknij czytnik w zasięgu metody i zwróć obiekt lub kolekcję klientowi.

Answer 7

I całkowicie zgadzam się z duffymo (+1) (i Bevan)

Pomysł I zostały żucia nad niedawno w moim umyśle jest, czy muszę mieć zależność, i oczywiście kiedy zmapować czytnik do obiektu muszę mieć zależność, może powinienem zrobić to całkowicie jednoznaczne i faktycznie napisać metodę rozszerzenia, aby go osiągnąć, coś jak ...

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

    } 
} 

w ten sposób, tak długo, jak jestem w zakres referencyjny mojego obiektu biznesowego, mój datareader będzie teraz miał metodę dostosowaną do moich potrzeb ... pomysł prawdopodobnie musi zostać ulepszony bardziej, ale myślę, że byłoby to eleganckie.

Answer 8

Do oddzielnych warstwach biznesowych i danych używać WYDAJNOŚĆ

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

Przechodząc czytnik danych do konstruktora może być dyskusyjna, ale - o ile wiem - jest dobrze znane podejście. Ale ktokolwiek go używa, powinien przekazać abstrakcję, tj. Nie SqlDataReader, ale IDataReader. Jednak IDataReader nie jest optymalny; powinien to być IDataRecord, który zawiera wynik dla danego rekordu, zamiast pozwalać na iterację !!