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

10

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ć?

Arseni Mourzenko
źródło
7
Czy możesz argumentować, że każdy niezależny „blok zakresu” powinien zamiast tego być własną metodą lub funkcją?
Dan Pichelman
Powiedziałbym, że „często” nie jest tak dobrze wdrażany, jak mógłby być (ale prawdopodobnie nie jest to problem „projektowy”). Czasami z założenia jest to nieuniknione (na przykład wyjątek / zakres zasobów).
JustinC

Odpowiedzi:

35

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.

Doktor Brown
źródło
4
Ale dlaczego jest źle? Posiadanie wielu nazwanych funkcji jest również mylące.
Kris Van Bael
1
+1 Dokładnie, jeśli potrzebujesz zakresu, prawdopodobnie wykonujesz określoną akcję, którą można wyodrębnić do funkcji. Kris, nie chodzi o tworzenie wielu funkcji, chodzi o to, że gdy zdarzają się takie przypadki (gdzie zakres jednego bloku może kolidować z innym), zwykle należy użyć funkcji. Widzę to także w innych językach (a mianowicie JavaScript, z IIFE zamiast zwykłych „starych funkcji”) przez cały czas.
Benjamin Gruenbaum,
10
@KrisVanBael: „Posiadanie wielu nazwanych funkcji jest także mylące” - jeśli używasz nazw opisowych, jest zupełnie odwrotnie. Tworzenie zbyt dużych funkcji jest najważniejszym powodem, dla którego kod staje się trudny w utrzymaniu, a nawet trudniejszy do rozróżnienia.
Doc Brown
2
Jeśli jest tak wiele funkcji, że ich wyraźne nazewnictwo staje się problemem, być może niektóre z nich można przekształcić w nowy typ, ponieważ mogą one mieć znaczną niezależność od pierwotnej treści funkcji. Niektóre mogą być tak blisko powiązane, że można je wyeliminować. Nagle nie ma tak wielu funkcji.
JustinC
3
Nadal nie jestem przekonany. Często długie funkcje są naprawdę złe. Ale stwierdzenie, że każda potrzeba zakresu wymaga osobnej funkcji, jest dla mnie zbyt czarno-białe.
Kris Van Bael
3

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ąć.

mouviciel
źródło
2

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.

DeveloperArnab
źródło
0

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?

Dejan Stanič
źródło