Можно ли передавать DataReaders конструкторам? - PullRequest
5 голосов
/ 20 мая 2009

Я поддерживаю некоторый код на C # 2.0, и программист использует шаблон чтения коллекции бизнес-объектов, открывая DataReader и затем передавая его конструктору объекта. Я не вижу ничего явно плохого в этом, но мне плохо. Это нормально делать?

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);
}

Ответы [ 9 ]

5 голосов
/ 20 мая 2009

Передавая DataReader конструктору объекта, вы устанавливаете очень тесную связь между бизнес-объектом и выбранной вами технологией персистентности.

По крайней мере, такое тесное соединение затруднит повторное использование и тестирование; в худшем случае это может привести к тому, что бизнес-объекты будут слишком много знать о базе данных.

Устранить это не так уж сложно - вы просто переместили бы инициализацию объекта из конструктора в отдельный класс фабрики.

3 голосов
/ 20 мая 2009

Передача читателя заставляет меня съеживаться, если только это не непубличный вспомогательный метод для копирования одной строки. Определенно не для конструктора.

Передача считывателя соединяет ваш конструктор (вы не помещали MyObject в класс, но вы вызываете new MyObject()) с вашим хранилищем данных, и я предполагаю, что ваш объект не написан таковым?

Если бы это был я:

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{}
1 голос
/ 20 мая 2009

Я бы назвал это "утечка абстракции".

Мне нравится использовать постоянные объекты в максимально узкой области: приобретать их, использовать их, очищать их. Если был четко определенный объект персистентности, вы можете попросить его отобразить результат запроса в объект или коллекцию, закрыть программу чтения в области действия метода и вернуть объект или коллекцию вашему клиенту.

1 голос
/ 20 мая 2009

Я бы сделал сущность переданной в IDataReader, поскольку это помогает тестировать MyObject.

1 голос
/ 20 мая 2009

(РЕДАКТИРОВАТЬ: Этот ответ сосредоточен исключительно на «каковы последствия на относительно низком уровне», а не общие последствия для дизайна. Похоже, что другие ответы были рассмотрены, поэтому я не буду комментировать:)

Ну, определенно что-то не так с кодом, который вы дали, поскольку ничто не закрывает соединение, команду или считыватель. В частности, ваша строка назначения соединения обычно должна выглядеть следующим образом:

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

Вы вполне можете подумать, что это просто придирки, и что это просто пример кода, и очистка не важна - но "владение" ресурсами, которые требуют очистки, как раз и является проблемой передача DataReader конструктору.

Я думаю, что все в порядке, если вы документируете, кто впоследствии «владеет» читателем. Например, в Image.FromStream изображение впоследствии владеет потоком, и вам может не понравиться закрывать его самостоятельно (в зависимости от формата изображения и некоторых других вещей). В других случаях это все еще ваша ответственность, чтобы закрыть. Это должно быть очень тщательно задокументировано, и если тип с конструктором становится владельцем, он должен реализовать IDisposable, чтобы упростить очистку и , чтобы сделать более очевидным, что очистка требуется.

В вашем случае, похоже, что конструктор не становится владельцем устройства чтения, что является прекрасной (и более простой) альтернативой. Просто задокументируйте это, и абоненту все равно придется соответствующим образом закрыть читателя.

1 голос
/ 20 мая 2009

Я бы так не поступил, но я не вижу в этом ничего особенного.

0 голосов
/ 11 ноября 2014

Передача чтения данных в конструктор может быть спорным, но - насколько я знаю, - это хорошо известный подход. Но тот, кто его использует, должен передавать абстракцию, т.е. не SqlDataReader, а IDataReader. И все же IDataReader не является оптимальным; вместо этого должен быть IDataRecord, который содержит результат для соответствующей записи, а не позволяет повторять !!

0 голосов
/ 23 июня 2014

Для разделения бизнес-уровней и слоев данных используйте YIELD

  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();
        }
    }
}
0 голосов
/ 20 мая 2009

Я полностью согласен с Даффимо (+1) (и Беваном)

Идея, которую я недавно размышлял в своем уме, заключается в том, что если у меня действительно есть зависимость, и, конечно, когда я сопоставляю читателя с объектом, у меня должна быть зависимость, возможно, я должен сделать ее полностью явной и на самом деле написать метод расширения для достижения этого, что-то вроде ...

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

  }
}

Таким образом, до тех пор, пока я нахожусь в контрольной области моего бизнес-объекта, мой источник данных теперь будет иметь метод, приспособленный к моим потребностям ... идею, вероятно, нужно конкретизировать, но я думаю, что это будет элегантный.

...