Czy używanie ELSE jest złym programowaniem? [Zamknięte]

Często spotykam błędy, które zostały spowodowane przez użycie ELSEkonstruktu. Doskonałym przykładem jest coś w stylu:

If (passwordCheck() == false){
    displayMessage();
}else{
    letThemIn();
}

Dla mnie to krzyczy problem bezpieczeństwa. Wiem, że PasswordCheck może być logiczną logiką, ale nie umieszczałbym na nim zabezpieczeń moich aplikacji. Co by się stało, gdyby to był ciąg znaków, int itp?

Zwykle staram się unikać używania ELSE, a zamiast tego wybieram dwie całkowicie oddzielne instrukcje IF, aby sprawdzić, czego oczekuję. Cokolwiek innego wtedy zostanie zignorowane LUB zostanie specjalnie potraktowane.

Z pewnością jest to lepszy sposób na zapobieganie błędom / problemom z bezpieczeństwem w aplikacji.

Jak to robicie?

elseBlok zawsze powinien składać się z co chcesz domyślne zachowanie się.

Nie musisz ich unikać, po prostu używaj ich odpowiednio.

W twoim przykładzie domyślnym stanem powinno być uniemożliwienie dostępu. Trochę refaktoryzacji pozostawia Ci:

If (passwordCheck)
{
   letThemIn();
}
else
{
   displayMessage();
}

tzn. jeśli sprawdzanie hasła działa, wpuść je, w przeciwnym razie zawsze jest ważny komunikat o błędzie.

Możesz oczywiście dodać dodatkowe kontrole do swojej logiki, używając else ifraczej niż całkowicie osobnegoif instrukcji.

Nie, nie ma w tym nic złego ELSE. ELSEnie jest nowy GOTO. W rzeczywistości, używając dwóch IFs zamiastELSE może prowadzić do kilku problemów.

Przykład pierwszy:

if (this(is(a(very())==complex(check())))) {
   doSomething();
}

if (!this(is(a(very())==complex(check())))) {
   doTheOtherThing();
}

Widzisz kopiowanie-wklej? Po prostu czeka na dzień, w którym zmienisz jedno i zapomnisz o drugim.

Przykład drugi:

foreach(x in y) {
  if (first_time) {
    first_time = false;
    doSomething();
  }

  if (!first_time) {
    doTheOtherThing();
  }
}

Jak widać, drugi IFzostanie również wykonany dla pierwszego elementu, ponieważ warunek już się zmienił. W rzeczywistych programach takie błędy są trudniejsze do wykrycia.

Zawsze jest ELSE. Jeśli napiszesz

if(foo)
  bar();

faktycznie piszesz

if(foo)
{
  bar();
}
else
{
   // do nothing
}

Wszystko, co umieścisz na ścieżce ELSE, spoczywa na tobie.

Osobiście staram się unikać elsejak najwięcej, ale to nie jest dla bezpieczeństwa problemu z .

Podczas odczytywania kodu zagnieżdżone instrukcje utrudniają przestrzeganie logiki, ponieważ należy pamiętać, który zestaw warunków tam doprowadzi. Z tego powodu jestem wielkim fanem wczesnego wyjścia:

if (checkPassword != OK) { displayMessage(); return; }

letThemIn();

Dotyczy to również fori whilepętle, w których będę używał continueibreak kiedy unika poziom wcięcia.

Chris Lattner mówi to lepiej niż ja w standardach kodowania LLVM .

Następnie po prostu je zastąp

If (passwordCheck == true)
{
     letThemIn();
}
else
{
     displayMessage();
}

Podobnie jak Matthieu M., wolę wcześniejsze wyjście niż głęboko zagnieżdżone inne bloki ... To dobrze ilustruje programowanie obronne (jeśli złe warunki, nie ma sensu kontynuować). Wiele osób się z nami nie zgadza, preferując wyjątkowy punkt wyjścia; nie o to chodzi w debacie (tak myślę).

Teraz z pewnością używam, elsegdy ma to sens, szczególnie w przypadku prostych, krótkich alternatyw. Jak powiedziano, powielanie testu jest stratą czasu (programisty i procesora), źródłem zamieszania, a później błędów (gdy jedno się zmienia, a nie drugie).
Czasami dodaję komentarz do elseczęści, przypominając o tym, co było warunkiem (szczególnie jeśliif część jest długa, np. W starszym kodzie) lub jaka jest alternatywa.

Zauważ, że niektórzy skrajni zwolennicy programowania funkcjonalnego proponują się go całkowicie pozbyć if, na rzecz dopasowania wzorców ... Trochę zbyt ekstremalnie jak na mój gust. :-)

Nie ma nic złego w korzystaniu z ELSE. Może to jednak prowadzić do zbyt skomplikowanego kodu, który jest trudny do odczytania i zrozumienia. Może to oznaczać zły projekt. Z pewnością oznacza to dodatkowe przypadki użycia, które będą musiały zostać przetestowane.

Spróbuj usunąć ELSE, jeśli możesz - ale nie bądź na to paranoikiem. Steve McConnell nazywa ten kod linii prostej w Code Complete. Tj. Jest prosta prosta ścieżka przez twój kod.

Podejścia do wypróbowania konkretnego problemu:

• stosować polimorfizm. Na granicy systemu sprawdź poprawność poświadczeń bezpieczeństwa użytkownika. Jeśli są uzasadnione, zwróć obiekt sesji - z dostępem do odpowiednich części systemu lub wyrzuć wyjątek. Może to jednak uczynić twój system bardziej złożonym. Więc decydujesz, co jest łatwiejsze do zrozumienia i utrzymania.

Ogólnie rzecz biorąc - poniższe elementy mogą pomóc w ograniczeniu ELSE w kodzie:

• dobre wymagania mogą zmniejszyć potrzebę takich decyzji w kodzie. W niektórych przypadkach może nie być konieczne wdrożenie przypadku użycia.
• wyraźniejszy projekt. Maksymalna spójność i minimalizacja sprzężenia. Dzięki temu komponenty nie duplikują decyzji podjętych w innych komponentach.
• obsługa wyjątków w celu zarządzania przypadkami błędów.
• polimorfizm (patrz przykład powyżej).
• instrukcje switch - są to gloryfikowane ELSE, ale w niektórych sytuacjach są lepsze.

Twoje przypuszczenie, że kod jest wyciekiem zabezpieczeń, może być lub nie być prawdziwe w zależności od używanego języka . W kodzie C może to być problem (szczególnie dlatego, że w C boolean jest tylko liczbą całkowitą niezerową lub zerową) - ale w najbardziej typowych językach (tj. Sprawdzanie typu środowiska wykonawczego), jeśli passwordCheckzmienna została zadeklarowana jako boolean, nie ma sposobu, aby przypisać coś innego. W rzeczywistości wszystko w ifpredykacie musi zostać przetworzone na wartość logiczną, niezależnie od tego, czy używasz operatorów boolowskich, czy po prostu używasz wartości. Jeśli udało się powiązać inny typ obiektupasswordCheck środowiskiem wykonawczym, wygenerowałby jakiś rodzaj nielegalnego wyjątku rzutowania.

Proste konstrukcje if / else są znacznie łatwiejsze do odczytania niż konstrukcje if / if - i mniej podatne na nieumyślne problemy, jeśli ktoś spróbuje przerzucić konstrukt. Weźmy ten sam przykład przez sekundę:

if(passwordCheck == false) {
    denyAccess();
}

if(passwordCheck) {
    letThemIn();
}

Znaczenie wzajemnie wykluczających się klauzul, które chcesz wykonać powyżej, zostało utracone. To właśnie przekazuje konstrukcja if / else. Dwie wzajemnie wykluczające się gałęzie egzekucji, przy czym jedna z nich zawsze będzie działać. Jest to ważna część bezpieczeństwa - upewnienie się, że nie ma sposobu, aby letThemInzadzwonić denyAccess.

W celu zapewnienia przejrzystości kodu oraz w celu zapewnienia największej ochrony sekcji krytycznych powinny one znajdować się w klauzuli podstawowej ( ifczęści). Domyślne zachowanie niezgodne powinno znajdować się w alternatywnej klauzuli ( elseczęści). Na przykład:

if(passwordCheck) {
    letThemIn();
} else {
    denyAccess();
}

UWAGA: pracując z różnymi językami, opracowałem nawyk kodowania, który pomaga uniknąć pytania „co jeśli to ciąg znaków?” Zasadniczo należy umieścić stałą na pierwszym miejscu w wyrażeniu boolowskim. Na przykład zamiast sprawdzaniapasswordCheck == false sprawdzam false == passwordCheck. Pozwala to również uniknąć przypadkowego problemu przypisania możliwego w C ++. Przy takim podejściu kompilator będzie narzekał, jeśli piszę =zamiast ==. W językach takich jak Java i C # kompilator traktowałby przypisanie w klauzuli if jako błąd, ale C ++ chętnie je zaakceptuje. Dlatego też mam tendencję do sprawdzania wartości zerowej przy nullpierwszym.

Jeśli rutynowo zmieniasz języki, ustawianie stałej jako pierwszej jest bardzo pomocne. Jednak w moim zespole jest to sprzeczne ze standardem kodowania i kompilator i tak wychwytuje te problemy. Przełamanie może być trudne.

Mówiąc to za pomocą else podczas programowania jest złe, jest jak mówienie, że używanieotherwise podczas mówienia jest złe.

Oczywiście, oba mogą być używane w zły sposób, ale to nie znaczy, że należy ich unikać tylko dlatego, że popełniłeś błąd, który ich obejmował. Nie zdziwiłbym się, gdyby wiele błędów zależało od brakującej defaultsprawy w switchoświadczeniu.

Pomyśl o Elsebiałej liście przepływu aplikacji. Sprawdzasz warunki, które POWINNY umożliwić kontynuowanie przepływu aplikacji, a jeśli nie są spełnione, toElse wykonywane jest zadanie rozwiązania problemu, przerwania wykonywania aplikacji lub coś podobnego.

Else samo w sobie nie jest złe, ale jeśli używasz go słabo, możesz zobaczyć niepożądane efekty.

Również w odniesieniu do twojego oświadczenia o

„Wiem, że sprawdzanie hasła może być logiczne, ale nie umieszczałbym na nim zabezpieczeń moich aplikacji”.

W przypadku opracowywanych metod ZAWSZE zwracaj jeden typ danych. Chociaż PHP Core jest zaśmiecony kodem, który zwraca dwa lub więcej typów danych, jest to zła praktyka, ponieważ powoduje zgadywanie wywołań funkcji. Jeśli musisz zwrócić więcej niż jeden typ danych, zastanów się nad rzuceniem wyjątku (uważam, że często jest to powód, dla którego chciałbym zwrócić inny typ danych - coś poszło okropnie, okropnie źle) lub rozważyć zmianę struktury kodu, abyś mógł zwraca tylko jeden typ danych.

Po pierwsze. LOL! Nie ma żadnego powodu, aby unikać w ogóle. NIE jest to zła praktyka w żaden sposób, w żadnym kształcie ani formie.

Jeśli cokolwiek, kod powinien być

if(!IsLoggedIn) { ShowBadLoginOrNoAccessPage(); return }

Nie ma dwóch, jeśli tam jest i nie ma nic innego. Tak robię we wszystkich moich aplikacjach oprócz tej, w której zgłaszam wyjątek. W mojej funkcji jest wychwytywany wyjątek, który sprawdza adres URL pod kątem wyświetlenia właściwej strony (lub alternatywnie mogę umieścić funkcję catch / check w funkcji błędu asp.net). Drukuje ogólną stronę z informacją, że nie autoryzuj lub jakąkolwiek wiadomość, której używam w wyjątku (zawsze sprawdzam typ wyjątku i ustawiam kod stanu HTTP).

-Edytuj- jak pokazano w przykładzie amunicji dwa, jeśli jest absurdalny. Naprawdę inne jest tak samo dobre lub lepsze niż if. Jeśli w ogóle należy tego unikać (chociaż ja osobiście tego nie robię, ale często używam return i break), ponieważ powiedziano, że więcej ścieżek kodu zwiększa prawdopodobieństwo błędów. Widzieć złożoność cykliczną

-Edycja 2- Jeśli martwisz się, czy użycie / else. Zauważę również, że wolę umieścić najkrótszy blok kodu na górze, taki jak

if(cond == false) {
    a()
    b()
    c()
    onetwothree()
}
else
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}

Zamiast

if(cond) 
{
    a()
    b()
    c()
    more()
    onetwothree()
    code()
    longer()
}
else
{
    a()
    b()
    c()
    onetwothree()
}

Kiedy mogę, mogę ustawić wartość domyślną przed warunkami warunkowymi. Wydaje mi się, że jest to trochę łatwiejsze do odczytania i bardziej jednoznaczne, ale to tylko preferencja. Mam tendencję do unikania negatywnych warunków w moim kodzie. Nie jestem wielkim fanem sprawdzania! Foo lub false == foo i wydaje mi się, że inaczej jest rodzajem warunkowego odpowiednika negatywu.

foo = bar;

if ('fubar' == baz) {
    foo = baz;
}

zamiast ...

if ('fubar' == baz) {
    foo = baz;
} else {
    foo = bar;
}

Poprzedni blok kodu wydaje mi się trochę łatwiejszy do odczytania. Bardziej naturalna wydaje mi się sceptyczna paranoja dotycząca mojego kodu. Ustawienie wartości domyślnej niezależnie od warunków sprawia, że ​​czuję się komfortowo: P

Twierdziłbym, że należy w jak największym stopniu unikać wszelkiego rodzaju logiki rozgałęziania. Chociaż nie ma nic złego w przypadku ELSE lub IF, istnieje wiele sposobów pisania kodu, aby zminimalizować potrzebę użycia dowolnej logiki rozgałęziającej. Nie twierdzę, że logikę rozgałęziania można całkowicie wyeliminować - będzie potrzebna w niektórych miejscach - ale możliwe jest przefakturowanie kodu, aby wyeliminować sporą jego część. W większości przypadków poprawi to zrozumiałość i dokładność kodu.

Na przykład operatorzy trójskładnikowi są zwykle dobrymi kandydatami:

If VIP Then 
  Discount = .25
Else
  Discount = 0
End If
Total = (1 - Discount) * Total

Stosując podejście trójskładnikowe:

Discount = VIP ? .25 : 0
Total = (1 - Discount) * Total

Operatorzy trójskładnikowi dobrze przesuwają rozgałęzienie w prawo.