Неправильно ли использовать фигурные скобки для целей переменной области видимости?

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

Collection<string> existingCategories = new Collection<string>();

// Here a beginning of a block
{
    SqlCommand getCategories = new SqlCommand("select Title from Movie.Category where SourceId = @sourceId", sqlConnection, sqlTransaction);
    getCategories.Parameters.AddWithValue("@sourceId", sourceId);
    using (SqlDataReader categoriesReader = getCategories.ExecuteReader(System.Data.CommandBehavior.SingleResult))
    {
        while (categoriesReader.Read())
        {
            existingCategories.Add(categoriesReader["Title"].ToString());
        }
    }
}

if (!existingCategories.Contains(newCategory))
{
    SqlCommand addCategory = new SqlCommand("insert into Movie.Category (SourceId, Title) values (@sourceId, @title)", sqlConnection, sqlTransaction);

    // Now try to make a mistake and write/copy-paste getCategories instead of addCategory. It will not compile.
    addCategory.Parameters.AddWithValue("@sourceId", sourceId);
    addCategory.Parameters.AddWithValue("@title", newCategory);
    addCategory.ExecuteNonQuery();
}

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

// Something like:
Collection<string> existingCategories = new Collection<string>();
{
    // Code here
}

// can be understood as (is it easy to notice that semicolon is missing?):
Collection<string> existingCategories = new Collection<string>()
{
    // Code here
}

So,

  1. Есть ли что-то неправильное в использовании фигурных скобок для создания блоков кода только для целей переменной области видимости?

  2. Если все в порядке, как сделать его более читабельным, не нарушая правил StyleCop?


person Arseni Mourzenko    schedule 06.07.2010    source источник
comment
Это так называемые анонимные блоки. Связанный: stackoverflow.com/questions/85282/ и stackoverflow.com/questions/500006/   -  person Michael Mrozek    schedule 06.07.2010


Ответы (5)


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

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

Что касается удобочитаемости, я могу понять, почему StyleCop это не нравится, но любой, у кого есть опыт работы с C/C++/Java/C#/..., знает, что пара фигурных скобок определяет область, и должно быть совершенно очевидно, что это то, что ты пытаешься сделать.

person Mac    schedule 06.07.2010

Нет ничего плохого само по себе в блокировке кода, но вам нужно подумать, почему вы это делаете.

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

person Adam Robinson    schedule 06.07.2010
comment
Я согласен. Технически нет ничего плохого. Это стилистическая жалоба и, возможно, предупреждающий знак о длинных функциях, которые действительно следует разделить на более мелкие и более управляемые части. - person Sean Edwards; 06.07.2010
comment
вы, вероятно, находитесь в ситуации, когда вам следует провести рефакторинг кода: речь идет не о копировании/вставке больших блоков кода. Та же проблема возникнет при использовании Intellisense, если две переменные имеют похожие имена (sqlGetProductFromTable против sqlAddProductToTable). Легко ошибиться в имени переменной, и, поскольку для компилятора код совершенно правильный, он выполнится, но выдаст что-то неожиданное. - person Arseni Mourzenko; 07.07.2010
comment
@MainMa: повторное использование блоков — не единственная причина рефакторинга. Огромный метод сам по себе может быть веской причиной для того, чтобы разбить его на более мелкие части. - person Adam Robinson; 07.07.2010

Используйте оператор using вместо голых фигурных скобок.

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

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

person Stephen Cleary    schedule 06.07.2010
comment
Я считаю, что для использования () требуется IDisposable, которого у него может не быть в этом контексте. - person Sean Edwards; 06.07.2010
comment
Это применимо в данном конкретном сценарии (поскольку SqlCommand реализует IDisposable, но не решит его общий случай. - person Adam Robinson; 06.07.2010
comment
Это некрасиво, но не могли бы вы использовать do { ... } while(false); вместо этого? Однако отключение предупреждения, вероятно, было бы лучшим решением. - person Adam Crume; 06.07.2010
comment
Я согласен. Превратите эти блоки кода в свои собственные методы и вызовите их. Блоки в том виде, в каком они есть у OP, выглядят немного странно, и я бы реорганизовал их, если бы встретил этот код. - person NotMe; 06.07.2010
comment
@ Джоэл Вы правы, в этом случае все было бы хорошо, но, как сказал Адам, это не относится к общему случаю, о котором я говорил. - person Sean Edwards; 06.07.2010

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

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

person STO    schedule 06.07.2010

Должен сказать, что если бы я работал над этим кодом после вас, меня бы немного расстроило ваше использование области видимости. Это не, афаик, обычная практика.

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

person Community    schedule 06.07.2010