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

10

В некоторых (довольно редких) случаях существует риск:

  • повторное использование переменной, которая не предназначена для повторного использования (см. пример 1),

  • или используя переменную вместо другой, семантически близко (см. пример 2).

Пример 1:

var data = this.InitializeData();
if (this.IsConsistent(data, this.state))
{
    this.ETL.Process(data); // Alters original data in a way it couldn't be used any longer.
}

// ...

foreach (var flow in data.Flows)
{
    // This shouldn't happen: given that ETL possibly altered the contents of `data`, it is
    // not longer reliable to use `data.Flows`.
}

Пример 2:

var userSettingsFile = SettingsFiles.LoadForUser();
var appSettingsFile = SettingsFiles.LoadForApp();

if (someCondition)
{
    userSettingsFile.Destroy();
}

userSettingsFile.ParseAndApply(); // There is a mistake here: `userSettingsFile` was maybe
                                  // destroyed. It's `appSettingsFile` which should have
                                  // been used instead.

Этот риск можно уменьшить, введя область действия:

Пример 1:

// There is no `foreach`, `if` or anything like this before `{`.

{
    var data = this.InitializeData();
    if (this.IsConsistent(data, this.state))
    {
        this.ETL.Process(data);
    }
}

// ...

// A few lines later, we can't use `data.Flows`, because it doesn't exist in this scope.

Пример 2:

{
    var userSettingsFile = SettingsFiles.LoadForUser();

    if (someCondition)
    {
        userSettingsFile.Destroy();
    }
}

{
    var appSettingsFile = SettingsFiles.LoadForApp();

    // `userSettingsFile` is out of scope. There is no risk to use it instead of
    // `appSettingsFile`.
}

Это выглядит неправильно? Вы бы избежали такого синтаксиса? Сложно ли это понять начинающим?

c# coding-style language-features scope Арсений Мурзенко
источник
7
Не могли бы вы утверждать, что каждый независимый «блок области видимости» должен вместо этого быть своим собственным методом или функцией?
Дан Пичельман
Я бы сказал, что «часто» не так хорошо реализовано, как могло бы быть (но, вероятно, это не проблема «дизайна»). Иногда по замыслу это неизбежно (например, исключение / определение ресурса).
JustinC

Ответы:

35

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

Чтобы подкрепить это своим личным опытом: несколько лет назад я унаследовал унаследованный проект C ++ с ~ 150 тыс. Строк кода, и он содержал несколько методов, использующих именно эту технику. И угадайте, что - все эти методы были слишком длинными. Поскольку мы реорганизовали большую часть этого кода, методы стали все меньше и меньше, и я почти уверен, что больше не осталось методов "внутренней области видимости"; они просто не нужны.

Док Браун
источник
4
Но почему это плохо? Наличие многих именованных функций также сбивает с толку.
Крис Ван Баел
1
+1 Точно, если вам нужна область, скорее всего, вы выполняете определенное действие, которое можно извлечь из функции. Крис, дело не в том, чтобы создавать множество функций, а в том, что когда возникают такие случаи (когда область действия одного блока может конфликтовать с областью действия другого), обычно следует использовать функцию. Я вижу это и в других языках (а именно в JavaScript, с IIFE вместо простых «старых» функций).
Бенджамин Грюнбаум
10
@KrisVanBael: «Наличие множества именованных функций также сбивает с толку» - если вы используете описательные имена, все наоборот. Создание слишком больших функций - главная причина того, что код становится трудно поддерживать и еще сложнее расширять.
Док Браун
2
Если существует так много функций, что их именование становится проблемой, возможно, некоторые из них можно свернуть в новый тип, поскольку они могут иметь значительную независимость от исходного тела функции. Некоторые могут быть настолько тесно связаны, что их можно устранить. Внезапно, не так много функций.
JustinC
3
Все еще не убежден. Часто длинные функции действительно плохие. Но говорить, что каждая потребность в области требует отдельной функции, слишком чёрно-белое для меня.
Крис Ван Баел
3

Начинающему (и любому программисту) гораздо проще думать о:

  • не используя переменную, которая не имеет отношения

чем думать о:

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

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

mouviciel
источник
2

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

DeveloperArnab
источник
0

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

Существует один конкретный сценарий, OTOH, в C #, когда они имеют тенденцию всплывать - это когда используются конструкции switch-case. Ситуация очень хорошо объяснена в: C # оператор Switch с / без фигурных скобок ... какая разница?

Деян Станич
источник