Czy „innego” należy używać w sytuacjach, w których przepływ sterowania czyni go zbędnym?

18

Czasami natrafiam na kod podobny do poniższego przykładu (to, co dokładnie robi ta funkcja, jest poza zakresem tego pytania):

function doSomething(value) {
  if (check1(value)) {
    return -1;
  }
  else if (check2(value)) {
    return value;
  }
  else {
    return false;
  }
}

Jak widać, if, else ifi elseoświadczenia są używane w połączeniu z returnoświadczeniem. Dla zwykłego obserwatora wydaje się to dość intuicyjne, ale myślę, że bardziej eleganckie (z perspektywy programisty) byłoby upuszczenie else-s i uproszczenie kodu w następujący sposób:

function doSomething(value) {
  if (check1(value)) {
    return -1;
  }
  if (check2(value)) {
    return value;
  }
  return false;
}

Ma to sens, ponieważ wszystko, co następuje po returninstrukcji (w tym samym zakresie), nigdy nie zostanie wykonane, dzięki czemu powyższy kod będzie semantycznie równy pierwszemu przykładowi.

Który z powyższych elementów bardziej odpowiada dobrym praktykom kodowania? Czy są jakieś wady obu metod w zakresie czytelności kodu?

Edycja: W tym pytaniu podano jako odniesienie duplikat . Uważam, że moje pytanie dotyczy innego tematu, ponieważ nie pytam o unikanie duplikatów stwierdzeń przedstawionych w drugim pytaniu. Oba pytania mają na celu ograniczenie liczby powtórzeń, choć w nieco inny sposób.

nosorożec
źródło
14
elseJest mniejszym problemem, większym problemem jest oczywiście jesteś powrocie dwa typy danych z jednej funkcji, która sprawia, że API funkcji nieprzewidywalne.
Andy,
3
E_CLEARLY_NOTADUPE
kojiro,
3
@DavidPacker: co najmniej dwa; może być trzy. -1jest liczbą, falsejest wartością logiczną i valuenie jest tu podana, więc może to być dowolny typ obiektu.
Yay295,
1
@ Yay295 Miałem nadzieję, że valueto rzeczywiście liczba całkowita. Jeśli może to być cokolwiek, to jest jeszcze gorzej.
Andy,
@DavidPacker Jest to bardzo ważna kwestia, szczególnie w przypadku pytania o dobre praktyki. Zgadzam się z tobą i Yay295, jednak próbki kodu służą jako przykład do wykazania niepowiązanej techniki kodowania. Niemniej jednak pamiętam, że jest to ważna sprawa i nie należy jej pomijać w rzeczywistym kodzie.
nosorożec

Odpowiedzi:

23

Podoba mi się ten bez elsei oto dlaczego:

function doSomething(value) {
  //if (check1(value)) {
  //  return -1;
  //}
  if (check2(value)) {
    return value;
  }
  return false;
}

Ponieważ zrobienie tego nie zepsuło niczego, czego nie powinno było.

Nienawiść współzależności we wszystkich jej formach (w tym nazywanie funkcji check2()). Izoluj wszystko, co można odizolować. Czasami potrzebujesz, elseale nie widzę tego tutaj.

candied_orange
źródło
Generalnie wolę to również z podanego tutaj powodu. Zazwyczaj jednak umieszczam komentarz na końcu każdego ifbloku, } //elseaby wyjaśnić podczas czytania kodu, że jedynym zamierzonym wykonaniem po ifbloku kodu jest, jeśli warunek w ifinstrukcji jest fałszywy. Bez czegoś takiego, szczególnie jeśli kod w ifbloku staje się bardziej złożony, może nie być jasne dla tego, kto utrzymuje kod, że intencją było, aby nigdy nie przekroczyć ifbloku, gdy ifwarunek jest spełniony.
Makyen,
@Makyen podniosłeś dobry punkt. Ten, który starałem się zignorować. Uważam również, że należy coś zrobić po tym, ifaby wyjaśnić, że ta sekcja kodu została wykonana i struktura jest zakończona. Aby to zrobić, robię to, czego tu nie zrobiłem (ponieważ nie chciałem odwracać uwagi od głównego punktu PO, wprowadzając inne zmiany). Robię najprostszą rzecz, jaką potrafię, a wszystko to bez rozpraszania uwagi. Dodaję pusty wiersz.
candied_orange
27

Myślę, że to zależy od semantyki kodu. Jeśli twoje trzy przypadki są od siebie zależne, wyraźnie to zaznacz. Zwiększa to czytelność kodu i ułatwia zrozumienie dla każdego innego. Przykład:

if (x < 0) {
    return false;
} else if (x > 0) {
    return true;
} else {
    throw new IllegalArgumentException();
}

Tutaj wyraźnie zależysz od wartości xwe wszystkich trzech przypadkach. Jeśli pominąłbyś ostatni else, byłoby to mniej jasne.

Jeśli twoje sprawy nie są bezpośrednio od siebie zależne, pomiń to:

if (x < 0) {
    return false;
}
if (y < 0) {
    return true;
}
return false;

Trudno to pokazać w prostym przykładzie bez kontekstu, ale mam nadzieję, że rozumiesz.

André Stannek
źródło
6

Wolę drugą opcję (oddzielny ifsbez else ifi na początku return) ale to jest tak długo, jak bloki kodu są krótkie .

Jeśli bloki kodu są długie, lepiej użyć else if, ponieważ w przeciwnym razie nie miałbyś jasnego zrozumienia kilku punktów wyjścia, które ma kod.

Na przykład:

function doSomething(value) {
  if (check1(value)) {
    // ...
    // imagine 200 lines of code
    // ...
    return -1;
  }
  if (check2(value)) {
    // ...
    // imagine 150 lines of code
    // ...
    return value;
  }
  return false;
}

W takim przypadku lepiej jest użyć else if, przechowywać kod powrotu w zmiennej i mieć tylko jedno zdanie zwrotne na końcu.

function doSomething(value) {
  retVal=0;
  if (check1(value)) {
    // ...
    // imagine 200 lines of code
    // ...
    retVal=-1;
  } else if (check2(value)) {
    // ...
    // imagne 150 lines of code
    retVal=value;
  }
  return retVal;
}

Ale ponieważ należy dążyć do tego, aby funkcje były krótkie, powiedziałbym, aby były krótkie i wychodziły wcześniej niż w pierwszym fragmencie kodu.

Tulains Córdova
źródło
1
Zgadzam się z twoim założeniem, ale dopóki rozmawiamy o tym, jak powinien wyglądać kod, czy równie dobrze moglibyśmy nie tylko zgodzić się, że bloki kodu powinny być krótkie? Myślę, że gorzej jest mieć długi blok kodu niepodzielony na funkcje, niż używać innego, jeśli musiałbym wybrać.
kojiro,
1
Ciekawy argument. returnmieści się w 3 liniach, ifwięc nie różni się tak bardzo, jak else ifnawet po 200 liniach kodu. Pojedynczy styl powrót przedstawisz tutaj tradycja C i innych języków, które nie zgłasza błędów z wyjątkami. Chodziło o stworzenie jednego miejsca do czyszczenia zasobów. Języki wyjątków używają do tego finallybloku. Podejrzewam, że tworzy to również miejsce do umieszczenia punktu przerwania, jeśli twój debugger nie pozwoli ci umieścić go w funkcjach zamykających nawias klamrowy.
candied_orange
4
W ogóle nie podoba mi się to zadanie retVal. Jeśli możesz bezpiecznie wyjść z funkcji, zrób to natychmiast, bez przypisywania bezpiecznej wartości, która ma być zwrócona do zmiennej pojedynczego wyjścia z funkcji. Kiedy używasz pojedynczego punktu powrotu w naprawdę długiej funkcji, generalnie nie ufam innym programistom i ostatecznie przeglądam resztę funkcji również pod kątem innych modyfikacji wartości zwracanej. Szybka porażka i wczesny powrót to między innymi dwie zasady, według których żyję.
Andy,
2
Moim zdaniem jest jeszcze gorzej w przypadku długich bloków. Staram się wychodzić z funkcji tak wcześnie, jak to możliwe, bez względu na kontekst zewnętrzny.
Andy,
2
Jestem tutaj z DavidPacker. Styl pojedynczego powrotu zmusza nas do przestudiowania wszystkich punktów przypisania, a także kodu po punktach przypisania. Studiowanie punktów wyjścia w stylu wczesnego powrotu byłoby dla mnie lepsze, długie lub krótkie. Tak długo, jak język pozwala na ostateczny blok.
candied_orange
4

Używam obu w różnych okolicznościach. Podczas sprawdzania poprawności pomijam pozostałe. W przepływie kontrolnym używam innego.

bool doWork(string name) {
  if(name.empty()) {
    return false;
  }

  //...do work
  return true;
}

vs

bool doWork(int value) {
  if(value % 2 == 0) {
    doWorkWithEvenNumber(value);
  }
  else {
    doWorkWithOddNumber(value);
  }
}

Pierwszy przypadek brzmi bardziej tak, jakbyś najpierw sprawdzał wszystkie wymagania wstępne, usuwając je z drogi, a następnie przechodząc do właściwego kodu, który chcesz uruchomić. W związku z tym soczysty kod, który naprawdę chcesz uruchomić, należy bezpośrednio do zakresu funkcji; na tym właśnie polega ta funkcja.
Drugi przypadek brzmi bardziej tak, jakby obie ścieżki były poprawnymi kodami do uruchomienia, które są tak samo istotne dla funkcji, jak inne na podstawie pewnych warunków. Jako takie należą one do siebie w podobnych poziomach zakresu.

Altainia
źródło
0

Jedyną sytuacją, jaką kiedykolwiek widziałem, to naprawdę ważne, jest taki kod:

List<?> toList() {
    if (left != null && right != null) {
        Arrays.asList(merge(left, right));
    }
    if (left != null) {
        return Arrays.asList(left);
    }
    if (right != null) {
        return Arrays.asList(right);
    }
    return Arrays.asList();
}

Jest tu oczywiście coś nie tak. Problem polega na tym, że nie jest jasne, czy returnnależy je dodać, czy Arrays.asListusunąć. Nie można tego naprawić bez głębszej kontroli powiązanych metod. Możesz uniknąć tej dwuznaczności, zawsze stosując w pełni wyczerpujące bloki if-else. To:

List<?> toList() {
    if (left != null && right != null) {
        Arrays.asList(merge(left, right));
    } else if (left != null) {
        return Arrays.asList(left);
    } else if (right != null) {
        return Arrays.asList(right);
    } else {
        return Arrays.asList();
    }
}

nie będzie się kompilował (w językach sprawdzanych statycznie), chyba że w pierwszej kolejności dodasz wyraźny zwrot if.

Niektóre języki nie pozwalają nawet na pierwszy styl. Staram się używać go tylko w ściśle imperatywnym kontekście lub jako warunek wstępny w długich metodach:

List<?> toList() {
    if (left == null || right == null) {
        throw new IllegalStateException()
    }
    // ...
    // long method
    // ...
}
Banthar
źródło
-2

Trzy wyjścia z takiej funkcji są bardzo mylące, jeśli próbujesz postępować zgodnie z kodem i źle postępować.

Jeśli masz pojedynczy punkt wyjścia dla tej funkcji, pozostałe są wymagane.

var result;
if(check1(value)
{
    result = -1;
}
else if(check2(value))
{
    result = value;
}
else 
{
    result = 0;
}
return result;

W rzeczywistości idziemy jeszcze dalej.

var result = 0;
var c1 = check1(value);
var c2 = check2(value);

if(c1)
{
    result = -1;
}
else if(c2)
{
    result = value;
}

return result;

Wystarczająco słuszne, możesz argumentować za jednym wczesnym zwrotem przy sprawdzaniu poprawności danych wejściowych. Po prostu unikaj zawijania całej masy, jeśli logika w if.

Ewan
źródło
3
Ten styl pochodzi z języków z jawnym zarządzaniem zasobami. Potrzebny był jeden punkt, aby zwolnić przydzielone zasoby. W językach z automatycznym zarządzaniem zasobami pojedynczy punkt powrotu nie jest tak ważny. Powinieneś raczej skupić się na zmniejszeniu liczby i zakresu zmiennych.
Banthar,
Odp .: w pytaniu nie podano języka. b: Myślę, że się mylisz w tej sprawie. istnieje wiele dobrych powodów do pojedynczego zwrotu. zmniejsza złożoność i poprawia czytelność
Ewan,
1
Czasami zmniejsza złożoność, czasem ją zwiększa. Zobacz wiki.c2.com/?ArrowAntiPattern
Robert Johnson
Nie, myślę, że to zawsze zmniejsza złożoność cyklomatyczną, patrz mój drugi przykład. check2 zawsze działa
Ewan,
wczesny powrót to optymalizacja, która komplikuje kod, przenosząc część kodu do warunkowego
Ewan,
-3

Wolę pominąć zbędną instrukcję else. Ilekroć to widzę (podczas przeglądania kodu lub refaktoryzacji), zastanawiam się, czy autor zrozumiał przepływ sterowania i czy w kodzie może być błąd.

Jeśli autor uważa, że ​​instrukcja else jest konieczna, a tym samym nie rozumie konsekwencji instrukcji return dla przepływu kontrolnego, to z pewnością taki brak zrozumienia jest głównym źródłem błędów, w tej funkcji i ogólnie.

Andreas Warberg
źródło
3
Chociaż akurat się z tobą zgadzam, nie jest to sonda opinii. Utwórz kopię zapasową swoich roszczeń, w przeciwnym razie wyborcy nie będą cię traktować uprzejmie.
candied_orange