Czy użycie wewnętrznych bloków zakresu w funkcji jest złym stylem?

Istnieje kilka (dość rzadkich) przypadków, w których istnieje ryzyko:

• ponowne użycie zmiennej, która nie jest przeznaczona do ponownego użycia (patrz przykład 1),

• lub używając zmiennej zamiast innej, semantycznie blisko (patrz przykład 2).

Przykład 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`.
}

Przykład 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.

Ryzyko to można ograniczyć, wprowadzając zakres:

Przykład 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.

Przykład 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`.
}

Czy to źle wygląda? Czy uniknąłbyś takiej składni? Czy początkującym trudno to zrozumieć?

Jeśli twoja funkcja jest tak długa, że ​​nie możesz już rozpoznać żadnych niepożądanych efektów ubocznych lub nielegalnego ponownego użycia zmiennych, to nadszedł czas, aby podzielić ją na mniejsze funkcje - co sprawia, że ​​wewnętrzny zakres nie ma sensu.

Aby to wesprzeć osobistym doświadczeniem: kilka lat temu odziedziczyłem starszy projekt C ++ z ~ 150 000 linii kodu i zawierał on kilka metod wykorzystujących dokładnie tę technikę. I zgadnij co - wszystkie te metody były zbyt długie. Ponieważ dokonaliśmy refaktoryzacji większości tego kodu, metody stawały się coraz mniejsze i jestem prawie pewien, że nie ma już żadnych metod „zakresu wewnętrznego”; po prostu nie są potrzebne.

Dla początkującego (i dowolnego programisty) o wiele łatwiej jest pomyśleć o:

• niestosowanie zmiennej, która nie jest istotna

niż myśleć o:

• dodawanie struktur kodu mających na celu zapobieganie nieużywaniu zmiennej, która nie jest istotna.

Co więcej, z punktu widzenia czytelnika, takie bloki nie mają widocznej roli: usunięcie ich nie ma wpływu na wykonanie. Czytelnik może podrapać się w głowę, próbując zgadnąć, co programista chciał osiągnąć.

Korzystanie z lunety jest technicznie właściwe. Ale jeśli chcesz, aby twój kod był bardziej czytelny, powinieneś wyodrębnić tę część w innej metodzie i nadać temu sensowi pełną nazwę. W ten sposób możesz podać zakres swoich zmiennych, a także podać nazwę swojego zadania.

Zgadzam się, że ponowne użycie zmiennej jest najczęściej zapachem kodu. Prawdopodobnie taki kod powinien być refaktoryzowany w mniejszych, samodzielnych blokach.

Istnieje jeden szczególny scenariusz, OTOH, w języku C #, gdy mają tendencję do wyskakiwania - to znaczy przy użyciu konstrukcji z przypadkami przełączania. Sytuacja jest bardzo ładnie wyjaśniona w: Instrukcja C # Switch z / bez nawiasów klamrowych… jaka jest różnica?