Это в порядке, чтобы передать DataReaders конструкторам?

Я бы не стал делать это таким образом, но я делаю не вижу ничего принципиально неправильного.

Я бы назвал это «дырявой абстракцией».

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

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

Передача средства чтения подключает ваш конструктор (вы не поместили 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{}

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

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

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

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

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

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

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

Я полностью согласен с Даффимо (+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)
  {

  }
}

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