03/08/2014

Czy użycie if zamiast else if ma znaczenie?


Historia zaczyna się od prostego fragmentu kodu pokazanego poniżej. Kod ten to fragment walidatora, ktory ma za zadanie określić, czy dane są prawidłowe. Jeśli nie, to zmienna isValid powinna zostać ustawiona na false.
var isValid = true;

if (condition_1)
   isValid = false;

if (condition_2)
   isValid = false;
Kod ten działał do momentu, kiedy wprowadzono do niego małą zmianę pokazaną poniżej. Było to pewne uszczegółowienie logiki walidacji danych wejściowych.
var isValid= true;

if (condition_1)
   isValid = false;

if (condition_2)
   isValid = false;
//Wprowadzona zmiana
else 
{
   if (condition_3)
      isValid = true;
   else if (condition_4)
      isValid = false;
}
Po zastanowieniu się z pewnością stwierdzicie, że coś tutaj nie pasuje. Problem polega na tym, że w określonych warunkach zmienna isValid może zostać z powrotem ustawiona na true nawet jeśli wcześniej stwierdzono, że dane są błędne i ustawiono ją na false!

Może są jakieś rzadkie i dziwne przypadki kiedy błędne dane mogą stać się nagle poprawne, ale w przypadku, z jakim się spotkałem, był to ewidentny błąd i przeoczenie albo brak zrozumienia działania kodu. Moim zdaniem błąd tkwi jednak w jeszcze jednym miejscu tj. w użyciu wielu if'ów zamiast konstrukcji else if w kodzie pokazanym na samym początku. Gdyby początkowy kod wyglądał w ten sposób:
var isValid = true;

if (condition_1)
   isValid = false;
// else if zamiast if
else if (condition_2)
   isValid = false;
To nawet jego modyfikacja w opisany wcześniej sposób nie spowodowałaby, że walidator pozwolił na kontynuację przetwarzania pomimo błędnych danych. Może wydawać się, że kilka if'ów zamiast użycia else if nie robi różnicy bo to przecież oczywiste jak ten kod działa. Może i oczywiste, ale tu i teraz. Za kilka tygodni lub miesięcy, dla kogoś innego, może już nie być takie oczywiste. Ktoś inny może również to po prostu przeoczyć.

Tworząc kod trzeba się starać, aby był możliwie łatwy do zrozumienia, rozszerzania i wprowadzania zmian, a w tym celu w przypadkach podobnych do opisanego wystarczy po prostu użycie trochę inne konstrukcji językowej, która lepiej wyrazi nasze zamiary innym.

5 comments:

  1. Dlatego ja unikam ustawiania w środku funkcji obydwu wartości boolowskich. W przypadku walidacji: true jest na początku, a później tylko się może zepsuć. Dodatkowo, jeżeli nie byłoby zapisywania przyczyny walidacji (domyślam się, że w przykładzie została usunięta dla przejrzystości) to można zrobić return. Jakoś nigdy się nie bałem (a znam takich co się boją) zrobić kilka returnów z funkcji (tylko w przypadku dość prostych funkcji).

    ReplyDelete
  2. Robię podobnie jak ty i dlatego to ustawienie isValid na true od razu rzuciło mi się w oczy. Jeśli chodzi o używanie return to ja nie widzę problemu aby użyć go kilka razy w jednej metodzie, w wielu przypadkach może to uprościć kod. Wszystko jest dla ludzi ;) jeśli stosowane z głową i umiarem.

    ReplyDelete
  3. Nie rozumiem problemu. Programista dla roznych blokow If stosuje przypisanie wartosci boolowskiej do tej samej zmiennej i dziwi sie, ze moze zostac ona nadpisana w kolejnym bloku? To sa raczej bledy poczatkujacego programisty.

    ReplyDelete
  4. Nikt się nie dziwi, że zmienna może być nadpisana w kolejnym bloku. W tym poście zwracam uwagę, że użycie odpowiednich konstrukcji językowych może lepiej pokazać co kod ma robić. "else if" jasno mówi: ma się wykonać tylko jeden blok. W przypadku kilku if'ów nie możesz być tego pewien.

    ReplyDelete
  5. Pierwszy kod jest też nieoptymalny (co może być bardzo kosztowne w niektórych przypadkach) ponieważ sprawdza więcej warunków niż musi.
    A tak przy okazji, gdyby to była osobna metoda ja robiłbym return-a po każdy sprawdzeniu.

    ReplyDelete