Как быстро я должен закрыть блок использования? - PullRequest
4 голосов
/ 31 марта 2011

Во время обзора кода на днях возник вопрос о том, как быстро должен быть закрыт блок использования. Один лагерь сказал: «Как только вы закончите с объектом»; другой - «когда-нибудь, прежде чем он выйдет за рамки».

В этом конкретном примере есть объект DataTable и объект SqlCommand, которые должны быть удалены. Нам нужно ссылаться на оба в одном выражении, и нам нужно перебрать DataTable.

Лагерь 1:

List<MyObject> listToReturn = new List<MyObject>();
DataTable dt = null;
try
{
    using (InHouseDataAdapter inHouseDataAdapter = new InHouseDataAdapter())
    using (SqlCommand cmd = new SqlCommand())
    {
        dt = inHouseDataAdapter.GetDataTable(cmd);
    }

    foreach (DataRow dr in dt.Rows)
    {
        listToReturn.Add(new MyObject(dr));
    }
}
finally
{
    if (dt != null)
    {
        dt.Dispose();
    }
}

Причина: утилизируйте SqlCommand, как только вы закончите с ним. Не запускайте потенциально длинные операции, такие как итерации таблицы, внутри блока using другого объекта.

Лагерь 2:

List<MyObject> listToReturn = new List<MyObject>();
using (InHouseDataAdapter inHouseDataAdapter = new InHouseDataAdapter())
using (SqlCommand cmd = new SqlCommand())
using (DataTable dt = inHouseDataAdapter.GetDataTable(cmd))
{
    foreach (DataRow dr in dt.Rows)
    {
        listToReturn.Add(new MyObject(dr));
    }
}

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

Я в Лагере 2. Где ты и почему?

Редактировать: Несколько человек указали, что DataTable не нужно утилизировать (см. ответ Кори Санволд ) и что оригинальный пример в Лагере 1 более уродлив, чем должен быть. Вот некоторые пересмотренные примеры, которые также учитывают тот факт, что большую часть времени мне приходится устанавливать некоторые свойства в SqlCommand. Если кто-то видел или может придумать лучший пример в поддержку какой-либо позиции, поделитесь им.

Лагерь 1, версия 2:

DataTable dt = null;
using (InHouseDataAdapter inHouseDataAdapter = new InHouseDataAdapter(_connectionString))
using (SqlCommand cmd = new SqlCommand("up_my_proc"))
{
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.Add("@class_id", 27);
    dt = inHouseDataAdapter.GetDataTable(cmd);
}

foreach (DataRow dr in dt.Rows)
{
    listToReturn.Add(new MyObject(dr));
}

Лагерь 2, версия 2:

using (InHouseDataAdapter inHouseDataAdapter = new InHouseDataAdapter(_connectionString))
using (SqlCommand cmd = new SqlCommand("up_my_proc"))
{
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.Add("@class_id", 27);
    DataTable dt = inHouseDataAdapter.GetDataTable(cmd);
    foreach (DataRow dr in dt.Rows)
    {
        listToReturn.Add(new MyObject(dr));
    }
}

Я думаю, что большинство людей согласятся с тем, что аргумент читабельности сейчас значительно уменьшен, и что это не лучший пример того, что я пытаюсь спросить. (Это особенно верно, если я скажу вам, что SqlConnection закрывается до выхода из метода GetDataTable (), и нет никаких ощутимых различий в производительности для данных, используемых в этом экземпляре.) Если я могу добавить к своему вопросу так поздно, есть ли случаи, когда это имеет значение, могу ли я избавиться от объекта немедленно? Например, , как упомянул Грегори Хигли, , общий ресурс, похожий на дескриптор ОС.

Редактировать: (Объясняя мой выбор ответа) Большое спасибо всем, кто высказал свое мнение, примеры и другие полезные отзывы! Кажется, мы разделены примерно на равные, но что следует из ответов каждого, так это идея, что «лагерь 1 определенно прав, но в зависимости от цели, лагерь 2 может быть в порядке». Я имел в виду, что это будет общее обсуждение избавления от всех типов объектов, но я выбрал плохой пример, чтобы проиллюстрировать это. Поскольку большая часть обсуждения была сосредоточена на этом конкретном примере, я выбрал ответ, который дал мне важную информацию о конкретных используемых объектах, и доказал, что мне нужно тщательно учитывать каждый объект при принятии такого рода решения. (В любом случае, было бы трудно выбрать «лучший ответ» на вопрос, столь же расплывчатый, как и мой заголовок.) Будущие читатели с такой же дилеммой, пожалуйста, посмотрите все ответы ниже, так как многие из них поднимают интересные вопросы.

Ответы [ 9 ]

3 голосов
/ 31 марта 2011

В основном я согласен с лагерем 1. Тем не менее, вы должны заметить, что не обязательно использовать DataTable .

.
1 голос
/ 31 марта 2011

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

Если неуправляемый ресурс каким-либо образом используется совместно, особенно если он представляет собой дескриптор операционной системы, я бы попытался утилизировать его как можно быстреенасколько возможно.

0 голосов
/ 31 марта 2011

Другим возможным вариантом было бы закрытие inHouseDataAdapter или SqlCommand в их блоках «Using», так как надлежащая реализация IDisposable должна быть терпимой к нескольким попыткам очистки. Во многих ситуациях я думаю, что явное закрытие / удаление в блоке Using может быть хорошей идеей, поскольку явно вызванный метод Close может предоставить более полезные исключения, чем IDisposable.Dispose (аргументы против IDisposable.Dispose, вызывающие исключение, не применить к методам Close).

В этом конкретном сценарии я бы утвердительно оставил SqlCommand и inHouseDataAdapter открытыми, однако при копировании DataTable в список. Если GetDataTable возвращает DataTable, который на самом деле содержит данные, цикл foreach должен выполняться быстро (чтобы объекты подачи данных не оставались открытыми слишком долго). Только если он возвращает производное DataTable с отложенным чтением, цикл foreach будет выполняться некоторое время, и в этом сценарии сущности источника данных, вероятно, должны будут оставаться открытыми достаточно долго, чтобы цикл завершился.

0 голосов
/ 31 марта 2011

Ваш пример в Лагере 1 немного похож на Соломенного Человека, он не должен быть таким уродливым.

Если производительность была проблемой (что, вероятно, редко, и вряд ли в этом надуманном примере)), Я бы выбрал более чистую версию «Лагеря 1» путем рефакторинга генерации DataTable в метод:

private DataTable GetDataTableFromInHouseAdapter()
{
    using (InHouseDataAdapter inHouseDataAdapter = new InHouseDataAdapter())
    using (SqlCommand cmd = new SqlCommand())
    {
        return inHouseDataAdapter.GetDataTable(cmd);
    }
}

...
List<MyObject> listToReturn = new List<MyObject>();
using (DataTable dt = GetDataTableFromInHouseAdapter())
{
    foreach (DataRow dr in dt.Rows)
    {
        listToReturn.Add(new MyObject(dr));
    }
}

Это выглядит более реалистично - метод, который генерирует DataTable, принадлежит в доступе к даннымСлой, и преобразование в список экземпляров MyObject, вероятно, принадлежит Фасаду над DAL.

На самом деле я бы всегда рассматривал рефакторинг вложенного оператора using в его собственный метод - за исключением случаев, когда они тесно связаны (например, SqlConnection / Command или даже InHouseDataAdapter / SqlCommand).

0 голосов
/ 31 марта 2011

+ 1 для лагеря 2.

Ваш образец в Лагере 1 располагает объектами, которые можно использовать ПОСЛЕ утилизации. Я думаю, что это не проблема с вашим конкретным сценарием, но может быть проблема с другими случаями. Версия Camp 2 вынуждает вас размещать объекты в правильно вложенных областях, делая код безопасным.

Пример:

StreamWriter writer;
using(var stream = new FileStream(name))
{
    writer = new StreamWriter(stream);
}
writer.Write(1); // <= writnig to closed stream here.
writer.Dispose();
0 голосов
/ 31 марта 2011

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

Но на самом деле это немного чистейший вопрос, поскольку мы говорим здесь о миллисекундах. Я бы сказал «ошибка» на стороне лагеря 1, особенно если вы долго используете запрошенные данные.

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

0 голосов
/ 31 марта 2011

Camp2 более читабелен, поэтому в большинстве случаев я бы предпочел его над Camp1.Чем более читабелен ваш код, тем меньше боли вы испытываете при его поддержке.

В некоторых редких случаях я бы предпочел Camp1, если возникнет очевидная необходимость очень быстро утилизировать ресурс.Я имею в виду, что если у вас возникли проблемы с удалением соединения чуть позже.Но в большинстве случаев не будет никакого штрафа, если вы пойдете по пути Camp2.

0 голосов
/ 31 марта 2011

Обычно мы используем лагерь 2 - по той причине, что оператор внешнего использования использует объект SqlTransaction.

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

0 голосов
/ 31 марта 2011

Я в Лагере 2, как и вы.Иногда критичность ресурсов определяется доступными ресурсами на машине.Решение Camp 2 действительно верит в превентивные меры, когда убирайте предметы, как вы сделали, а не ленивый Camp 1.

...