Użyj innego po wyjątku (lub nie)

9

Rozważ ten fragment kodu:

if (x == 1)
{
  throw "no good; aborting" ;
}

[... more code ...]

Teraz rozważ ten kod:

if (x == 1)
{
  throw "no good; aborting" ;
}
else
{
  [... more code ...]
}

Oba przypadki działają dokładnie w ten sam sposób. Pierwszy przypadek ma tę zaletę, że nie trzeba „otaczać” reszty kodu w pliku else. Drugi ma tę zaletę, że postępuje zgodnie z praktyką jawnego posiadania elsedla każdego if.

Czy ktoś może przedstawić jakieś solidne argumenty na korzyść jednego nad drugim?

Rlandster
źródło
5
Praktyka, którą przytaczasz na rzecz tego jawnego, elsewydaje się nieprawdziwa. Dość często po prostu nie ma nic do włożenia w elseblok, chyba że się pochylisz.
Konsystencja? Zezwolić na odporność w obliczu zmiany kodu? Czytelność?
Thomas Eding
1
Ehhhh, nie jestem fanem pomysłu, że każdy ifpotrzebuje else. Ostatni programista, który pracował na naszej bazie kodu, przestrzegał tego sztywno ( no cóż, czasami ... to rodzaj schizofrenika ). W rezultacie mamy wiele całkowicie bezsensownych else { /* do nothing */ }bloków zaśmiecających kod ...
KChaloux
4
„Inna dla każdego, jeśli” wydaje się dziwną proklamacją architekta chmury w imię (głupiej) konsekwencji. Nie widzę korzyści z przestrzegania tej praktyki i nigdy o niej nie słyszałem.
Erik Dietrich
To jest zbędne. Jeśli pracujesz ze stosem .NET, zainstaluj ReSharper, aby przypomnieć Ci o usunięciu wszystkich zbędnych instrukcji else.
CodeART

Odpowiedzi:

16

Nie należy dodawać elsepo ifgałęziach, które bezwarunkowo przerywają przepływ sterowania , takich jak te zawierające a throwlub a return. Poprawiłoby to czytelność twojego programu, usuwając niepotrzebny poziom zagnieżdżania wprowadzony przez elsegałąź.

To, co wygląda mniej więcej tak samo z jednym, throwstaje się naprawdę brzydkie, gdy zostaje zaatakowanych kilka rzutów z rzędu:

void myMethod(int arg1, int arg2, int arg3) {
    // This is demonstrably ugly - do not code like that!
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    } else {
        if (!isValid(arg2)) {
            throw new ArgumentException("arg2 is invalid");
        } else {
            if (!isValid(arg3)) {
                throw new ArgumentException("arg3 is invalid");
            } else {
                // The useful code starts here
            }
        }
    }
}

Ten fragment kodu robi to samo, ale wygląda znacznie lepiej:

void myMethod(int arg1, int arg2, int arg3) {
    if (!isValid(arg1)) {
        throw new ArgumentException("arg1 is invalid");
    }
    if (!isValid(arg2)) {
        throw new ArgumentException("arg2 is invalid");
    }
    if (!isValid(arg3)) {
        throw new ArgumentException("arg3 is invalid");
    }
    // The useful code starts here
}
dasblinkenlight
źródło
+1 prawda. Drugi przypadek OP zmusza cię do uważnego przeczytania, a następnie pozostawia WTF. Ale ... zawsze staraj się skracać metody. Zwrot w połowie metody 200 linii jest również zły.
Tulains Córdova
1
Szczerze mówiąc, jeśli używasz powtarzanego, jeśli możesz else if.
Guvante
2
@Guvante: Każdy iftestuje pojedynczy warunek i radzi sobie z nim, jeśli warunek jest prawdziwy, i jeśli nie jest jakaś alternatywna rzecz, która musi się zdarzyć, jeśli warunek jest fałszywy, else ifs są niepotrzebne. W moim biurze jest termin na kod, taki jak pierwszy fragment dasblinkenlight: „ maszyny pachinko ”.
Blrfl
@Blrfl maszyny pachinko hah, idealna analogia +1
Jimmy Hoffa
@Blrfl: Odniosłem się do tego, że powtarzane ifs są złym przykładem zbyt dużego zagnieżdżenia. W każdym razie nie powinieneś zagnieżdżać się w powtarzanych powtórzeniach. Zgadzam się, że ogólnie, chyba że mówisz o niewielkiej ilości kodu, nie ma powodu, aby to uwzględniać else.
Guvante
5

Nazwałbym praktykę „wyraźne inaczej”, którą nazywacie anty-wzorcem, ponieważ przesłania fakt, że nie ma kodu specjalnego przypadku jako innego dla waszego if.

Czytelność / łatwość konserwacji jest ogólnie poprawiona, gdy przeważnie masz tylko niezbędne konstrukcje przepływu kodu i minimalizujesz je. Oznacza to nadmiarowe wady i if, które dodadzą zakres do całej funkcji, utrudniają śledzenie i utrzymanie jej.

Powiedz na przykład, że masz tę funkcję:

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
    }
    else
    {
        oblogonToConfigure.Color = _validColors[0];
        oblogonToConfigure.ColorIndex = 0;
    }
}

Teraz pojawia się wymóg, że podczas konfiguracji należy również określić indeks typu / typu oblogon, istnieje wiele zakresów, w których ktoś może umieścić ten kod i skończyć z nieprawidłowym kodem, tj.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validOblogons.Contains(oblogonToConfigure.Type))
    {
        oblogonToConfigure.Type = _validOblogons[0];
        oblogonToConfigure.TypeIndex = 0;
        if (_validColors.Contains(oblogonToConfigure.Color))
        {
            oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
        }
        else
        {
            oblogonToConfigure.Color = _validColors[0];
            oblogonToConfigure.ColorIndex = 0;
        }
    }
    else
    {
        oblogonToConfigure.TypeIndex = _validOblogons.IndexOf(oblogonToConfigure.Type);
    }
}

Porównaj to z tym, czy oryginalny kod został napisany z minimalnymi potrzebnymi i minimalnymi konstrukcjami kontroli przepływu.

public void ConfigureOblogon(Oblogon oblogonToConfigure)
{
    if (!_validColors.Contains(oblogonToConfigure.Color))
    {
        oblogonToConfigure.Color = _validColors[0];
    }

    oblogonToConfigure.ColorIndex = _validColors.IndexOf(oblogonToConfigure.Color);
}

Znacznie trudniej byłoby teraz przypadkowo umieścić coś w niewłaściwym zakresie lub skończyć rozdętymi zakresami, powodując dublowanie w długim okresie wzrostu i utrzymania tej funkcji. Ponadto jest oczywiste, jakie są możliwe przepływy przez tę funkcję, więc poprawiono czytelność.

Wiem, że ten przykład jest nieco wymyślony, ale wiele razy widziałem

SomeFunction()
{
    if (isvalid)
    {
        /* ENTIRE FUNCTION */
    }
    /* Nothing should go here but something does on accident, and an invalid scenario is created. */
}

Dlatego sformalizowanie tych reguł dotyczących konstrukcji kontroli przepływu może pomóc ludziom rozwinąć intuicję niezbędną do wyczuwania czegoś, kiedy zaczną pisać kod w ten sposób. Wtedy zaczną pisać ...

SomeFunction()
{
    if (!isvalid)
    {
        /* Nothing should go here, and it's so small no one will likely accidentally put something here */
        return;
    }

    /* ENTIRE FUNCTION */
}
Jimmy Hoffa
źródło