Czy inne bloki zwiększają złożoność kodu? [Zamknięte]

18

Oto bardzo uproszczony przykład . To niekoniecznie jest pytanie specyficzne dla języka i proszę zignorować wiele innych sposobów pisania funkcji i zmian, które można w niej wprowadzić. . Kolor jest wyjątkowego rodzaju

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    else
    {
        return "No you can't";
    }
}

Wielu ludzi, których spotkałem, ReSharper, i ten facet (którego komentarz przypomniał mi, że chciałem o to zapytać od dłuższego czasu) zaleciłby przeformułowanie kodu w celu usunięcia elsebloku, pozostawiając to:

(Nie pamiętam, co powiedziała większość, mógłbym nie zapytać inaczej)

string CanLeaveWithoutUmbrella()
{
    if(sky.Color.Equals(Color.Blue))
    {
        return "Yes you can";
    }
    return "No you can't";
}

Pytanie: Czy wprowadzono wzrost złożoności poprzez pominięcie elsebloku?

Mam wrażenie, że elsebardziej bezpośrednio określa intencje, stwierdzając, że kod w obu blokach jest bezpośrednio powiązany.

Dodatkowo uważam, że mogę zapobiec subtelnym błędom logicznym, szczególnie po późniejszych modyfikacjach kodu.

Weźmy tę odmianę mojego uproszczonego przykładu (ignorując fakt, że oroperator, ponieważ jest to celowo uproszczony przykład):

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

Ktoś może teraz dodać nowy if blok na podstawie warunku po pierwszym przykładzie bez natychmiastowego prawidłowego rozpoznania pierwszego warunku nałożenia ograniczenia na swój własny warunek.

Jeśli elseblok byłby obecny, ktokolwiek dodałby nowy warunek, byłby zmuszony przenieść zawartośćelse bloku (i jeśli w jakiś sposób go nadpisuje, heurystyka pokaże, że kod jest nieosiągalny, czego nie robi w przypadku ifograniczenia jednego) .

Oczywiście istnieją inne sposoby zdefiniowania konkretnego przykładu, z których wszystkie zapobiegają tej sytuacji, ale to tylko przykład.

Długość podanego przeze mnie przykładu może zniekształcać wizualny aspekt tego, więc załóżmy, że przestrzeń zajmowana przez nawiasy jest względnie nieistotna dla reszty metody.

Zapomniałem wspomnieć o przypadku, w którym zgadzam się z pominięciem bloku else, i kiedy używam ifbloku do zastosowania ograniczenia, które musi być logicznie spełnione dla całego następującego kodu, takiego jak kontrola zerowa (lub inne zabezpieczenia) .

Selali Adobor
źródło
W rzeczywistości, gdy istnieje wyrażenie „jeśli” z „zwrotem”, może być postrzegane jako „blok ochronny” w> 50% wszystkich przypadków. Myślę więc, że masz tutaj błędne przekonanie, że „blok ochronny” jest przypadkiem wyjątkowym, a twój przykład zwykłym przypadkiem.
Doc Brown,
2
@AssortedTrailmix: Zgadzam się, że taki przypadek może być bardziej czytelny podczas pisania elseklauzuli (według mojego gustu będzie jeszcze bardziej czytelny, pomijając niepotrzebne nawiasy. Myślę jednak, że przykład nie jest dobrze wybrany, ponieważ w powyższy przypadek napisałbym return sky.Color == Color.Blue(bez if / else). Przykład z innym typem zwrotu niż bool prawdopodobnie by to wyjaśnił
Doc Brown
1
jak wskazano w komentarzach powyżej, sugerowany przykład jest zbyt uproszczony, zachęcając do spekulacyjnych odpowiedzi. Jeśli chodzi o tę część pytania, która zaczyna się od „Ktoś może teraz dodać nowy blok if ...” , była już wielokrotnie zadawana i udzielana odpowiedź, patrz np. Podejścia do sprawdzania wielu warunków? i wiele powiązanych z nim pytań.
komar
1
Nie rozumiem, co jeszcze bloki mają wspólnego z zasadą OSUSZANIA. DRY ma więcej wspólnego z abstrakcją i odniesieniami do powszechnie używanego kodu w postaci funkcji, metod i obiektów niż z pytaniem. Twoje pytanie dotyczy czytelności kodu i ewentualnie konwencji lub idiomów.
Shashank Gupta,
2
Głosowanie w celu ponownego otwarcia. Dobre odpowiedzi na to pytanie nie są szczególnie oparte na opiniach. Jeśli podany przykład w pytaniu jest nieodpowiedni, poprawienie go przez edycję byłoby rozsądnym posunięciem, bez głosowania na zakończenie z powodu, który nie jest w rzeczywistości prawdą.
Michael Shaw,

Odpowiedzi:

27

Moim zdaniem preferowany jest wyraźny blok else. Kiedy to widzę:

if (sky.Color != Blue) {
   ...
}  else {
   ...
}

Wiem, że mam do czynienia z wzajemnie wykluczającymi się opcjami. Nie muszę czytać, co jest w blokach if, aby móc powiedzieć.

Kiedy to widzę:

if (sky.Color != Blue) {
   ...
} 
return false;

Na pierwszy rzut oka wygląda na to, że zwraca wartość false i ma opcjonalny efekt uboczny, że niebo nie jest niebieskie.

Moim zdaniem struktura drugiego kodu nie odzwierciedla tego, co faktycznie robi kod. To źle. Zamiast tego wybierz pierwszą opcję, która odzwierciedla logiczną strukturę. Nie chcę sprawdzać logiki powrotu / zerwania / kontynuacji w każdym bloku, aby poznać przepływ logiczny.

Dlaczego ktoś wolałby drugą, jest dla mnie tajemnicą, mam nadzieję, że ktoś, kto zajmie przeciwne stanowisko, oświeci mnie swoją odpowiedzią.

Zapomniałem wspomnieć o przypadku, w którym zgadzam się z pominięciem bloku else i jest to, gdy używam bloku if do zastosowania ograniczenia, które musi być logicznie spełnione dla całego następującego kodu, takiego jak kontrola zerowa (lub inne zabezpieczenia ).

Powiedziałbym, że warunki wartowników są w porządku, jeśli opuściłeś szczęśliwą ścieżkę. Wystąpił błąd lub awaria, która uniemożliwia kontynuowanie zwykłego wykonywania. Gdy tak się stanie, należy zgłosić wyjątek, zwrócić kod błędu lub cokolwiek odpowiedniego dla Twojego języka.

Wkrótce po oryginalnej wersji tej odpowiedzi w moim projekcie odkryto taki kod:

Foobar merge(Foobar left, Foobar right) {
   if(!left.isEmpty()) {
      if(!right.isEmpty()) {
          Foobar merged = // construct merged Foobar
          // missing return merged statement
      }
      return left;
   }
   return right;
}

Nie wprowadzając zwrotu do innych elementów, przeoczono fakt, że brakowało zwrotu. Gdyby zastosowano inne, kod nawet by się nie skompilował. (Oczywiście o wiele bardziej martwi mnie to, że testy napisane na tym kodzie były dość złe, aby nie wykryć tego problemu).

Winston Ewert
źródło
Podsumowując moje przemyślenia na ten temat, cieszę się, że nie jestem jedynym, który tak myśli. Pracuję z programistami, narzędziami i IDE preferującymi usuwanie elsebloku (może to być uciążliwe, gdy jestem zmuszony to zrobić, aby zachować zgodność z konwencjami dla bazy kodu). Ja też jestem zainteresowany zobaczeniem tutaj kontrapunktu.
Selali Adobor,
1
Różnię się od tego. Czasami wyraźne słowo „inny” nie przynosi zbyt wiele, ponieważ jest mało prawdopodobne, aby wywoływały subtelne błędy logiczne, o których wspomina OP - to po prostu hałas. Ale w większości się zgadzam i od czasu do czasu zrobię coś takiego, } // elsegdzie normalnie pójdzie jako kompromis między nimi.
Telastyn
3
@Telastyn, ale co zyskujesz, pomijając inne? Zgadzam się, że korzyść jest bardzo niewielka w wielu sytuacjach, ale nawet dla niewielkiej korzyści uważam, że warto to zrobić. Z pewnością wydaje mi się dziwne umieszczanie czegoś w komentarzu, gdy może to być kod.
Winston Ewert
2
Myślę, że masz takie samo nieporozumienie jak OP - „jeśli” z „powrotem” można postrzegać głównie jako blok ochronny, więc omawiasz tutaj wyjątkowy przypadek, podczas gdy częstszy przypadek bloków ochronnych jest IMHO bardziej czytelny bez "jeszcze".
Doc Brown,
... więc to, co napisałeś, nie jest złe, ale IMHO nie jest warte wielu pozytywnych opinii.
Doc Brown,
23

Głównym powodem usunięcia elsebloku, który znalazłem, jest nadmierne wcięcie. Zwarcie elsebloków umożliwia znacznie bardziej płaską strukturę kodu.

Wiele ifstwierdzeń jest w zasadzie strażnikami. Zapobiegają dereferencjowaniu wskaźników zerowych i innych „nie rób tego!” błędy. I prowadzą do szybkiego zakończenia bieżącej funkcji. Na przykład:

if (obj === NULL) {
    return NULL;
}
// further processing that now can assume obj attributes are set

Teraz może się wydawać, że elseklauzula byłaby tutaj bardzo rozsądna:

if (obj === NULL) {
    return NULL;
}
else if (obj.color != Blue) {
   // handle case that object is not blue
}

I rzeczywiście, jeśli to wszystko, co robisz, nie jest to dużo bardziej wcięte niż w powyższym przykładzie. Ale to rzadko wszystko, co robisz z prawdziwymi wielopoziomowymi strukturami danych (warunek ten występuje cały czas podczas przetwarzania popularnych formatów, takich jak JSON, HTML i XML). Jeśli więc chcesz zmodyfikować tekst wszystkich elementów potomnych danego węzła HTML, powiedz:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
p = elements[0]
if ((p.children === NULL) or (p.children.length == 0)) {
    return NULL;
}
for (c in p.children) {
    c.text = c.text.toUpperCase();
}
return p;

Strażnicy nie zwiększają wcięcia kodu. W przypadku elseklauzuli cała praca zaczyna się przesuwać w prawo:

elements = tree.xpath(".//p");
if (elements === NULL) {
    return NULL;
}
else {
    p = elements[0]
    if ((p.children === NULL) or (p.children.length == 0)) {
        return NULL;
    }
    else {
        for (c in p.children) {
            c.text = c.text.toUpperCase();
        }
        return p;
    }
}

To zaczyna mieć znaczący ruch w prawo, i jest to dość trywialny przykład, z tylko dwoma warunkami ochronnymi. Każdy dodatkowy strażnik dodaje kolejny poziom wcięcia. Prawdziwy kod Napisałem, że przetwarzam XML lub korzystam ze szczegółowych kanałów JSON, które mogą łatwo łączyć 3, 4 lub więcej warunków ochrony. Jeśli zawsze korzystasz zelse , skończysz wcięciem 4, 5 lub 6 poziomów. Może więcej. A to zdecydowanie przyczynia się do poczucia złożoności kodu i więcej czasu poświęconego na zrozumienie, co z nim jest zgodne. Szybki, płaski styl wczesnego wyjścia eliminuje część tej „nadmiarowej struktury” i wydaje się prostszy.

Dodatek Komentarze do tej odpowiedzi uświadomiły mi, że może nie być jasne, że NULL / puste obchodzenie się nie jest jedynym powodem warunku warunkowego. Nie blisko! Podczas gdy ten prosty przykład koncentruje się na obsłudze NULL / pustej i nie zawiera innych powodów, przeszukiwanie głębokich struktur, takich jak XML i AST, w celu znalezienia „odpowiednich” treści, na przykład, często obejmuje długą serię konkretnych testów w celu wyeliminowania nieistotnych węzłów i nieciekawych skrzynie Seria testów „jeśli ten węzeł nie jest istotny, zwróć” spowoduje taki sam dryf w prawo, jak NULL / puste osłony. W praktyce kolejne testy trafności są często łączone z wartościami NULL / pustymi zabezpieczeniami dla odpowiednio głębszych przeszukiwanych struktur danych - podwójny szum dla przesunięcia w prawo.

Jonathan Eunice
źródło
2
To jest szczegółowa i ważna odpowiedź, ale dodam ją do pytania (na początku uciekło mi to z głowy); Istnieje „wyjątek”, w którym zawsze uważam, że elseblok jest zbyteczny, gdy używam ifbloku do zastosowania ograniczenia, które musi być logicznie spełnione dla całego następującego kodu, takiego jak kontrola zerowa (lub inne zabezpieczenia).
Selali Adobor
@AssortedTrailmix Uważam, że jeśli wykluczysz przypadki, w których rutynowo możesz zrobić wcześniejsze wyjście z thenklauzuli (takie jak kolejne instrukcje zabezpieczające), to unikanie konkretnej elseklauzuli nie ma szczególnej wartości . Rzeczywiście, zmniejszyłoby to klarowność i potencjalnie byłoby niebezpieczne (nie podając po prostu alternatywnej struktury, która jest / powinna istnieć).
Jonathan Eunice,
Myślę, że uzasadnione jest użycie warunków wartowników, aby wysunąć się, gdy opuścisz szczęśliwą ścieżkę. Jednak w przypadku przetwarzania XML / JSON stwierdzam również, że jeśli najpierw zweryfikujesz dane wejściowe względem schematu, otrzymasz lepsze komunikaty o błędach i czystszy kod.
Winston Ewert
To doskonałe wyjaśnienie przypadków, w których unika się innych.
Chris Schiffhauer,
2
Owinąłbym interfejs API, aby zamiast tego nie produkować głupich wartości NULL.
Winston Ewert
4

Kiedy to widzę:

if(something){
    return lamp;
}
return table;

Widzę wspólny idiom . Wspólny wzór , który w mojej głowie przekłada się na:

if(something){
    return lamp;
}
else return table;

Ponieważ jest to bardzo powszechny idiom w programowaniu, programiści przyzwyczajeni do oglądania tego rodzaju kodu mają ukryte „tłumaczenie” w swojej głowie, ilekroć go zobaczą, co „konwertuje” je na właściwe znaczenie.

Nie potrzebuję tego wyraźnego else, moja głowa „umieszcza go tam” dla mnie, ponieważ rozpoznał wzór jako całość . Rozumiem znaczenie całego wspólnego wzorca, więc nie potrzebuję drobnych szczegółów, aby go wyjaśnić.

Na przykład przeczytaj następujący tekst:

wprowadź opis zdjęcia tutaj

Czytałeś to?

Uwielbiam Paryż wiosną

Jeśli tak, to się mylisz. Przeczytaj tekst jeszcze raz. To, co faktycznie napisano, to

Kocham Paryż w na wiosnę

dwa tekście słowa „the”. Dlaczego więc tęskniłeś za jednym z dwóch?

To dlatego, że twój mózg widział wspólny wzór i natychmiast przetłumaczył go na znane znaczenie. Nie trzeba było czytać całego detalu po szczegółach, aby zrozumieć znaczenie, ponieważ rozpoznał już wzór po jego zobaczeniu. Dlatego zignorował dodatkowe „the ”.

To samo z powyższym idiomem. Programiści, którzy już czytają dużo kodu są przyzwyczajeni do tego wzorca , z if(something) {return x;} return y;. Nie potrzebują wyjaśnienia, elseaby zrozumieć jego znaczenie, ponieważ ich mózg „umieszcza je dla nich”. Rozpoznają go jako wzorzec o znanym znaczeniu: „co będzie po nawiasie zamykającym, będzie działało tylko jako domniemanie elsepoprzedniego if”.

Moim zdaniem nie elsejest to konieczne. Ale po prostu użyj tego, co wydaje się bardziej czytelne.

Aviv Cohn
źródło
2

Dodają zaśmiecenie wizualne w kodzie, więc tak, mogą zwiększyć złożoność.

Jednak jest też odwrotnie, nadmierne refaktoryzowanie w celu skrócenia długości kodu może również zwiększyć złożoność (oczywiście nie w tym prostym przypadku).

Zgadzam się ze stwierdzeniem, że elseblok stwierdza zamiar. Ale mój wniosek jest inny, ponieważ dodajesz złożoność w kodzie, aby to osiągnąć.

Nie zgadzam się z twoim stwierdzeniem, że pozwala to uniknąć subtelnych błędów w logice.

Zobaczmy przykład, teraz powinien on zwrócić prawdę tylko wtedy, gdy niebo jest niebieskie i jest wysoka wilgotność, wilgotność nie jest wysoka lub jeśli niebo jest czerwone. Ale wtedy pomylisz jedną nawias klamrowy i umieszczasz go w niewłaściwym miejscu else:

bool CanLeaveWithoutUmbrella() {
    if(sky.Color == Color.Blue)
    {
        if (sky.Humidity == Humidity.High) 
        {
            return true;
        } 
    else if (sky.Humidity != Humidity.High)
    {
        return true;
    } else if (sky.Color == Color.Red) 
    {
            return true;
    }
    } else 
    {
       return false;
    }
}

Widzieliśmy wszystkie tego rodzaju głupie błędy w kodzie produkcyjnym i mogą być bardzo trudne do wykrycia.

Sugerowałbym następujące:

Uważam to za łatwiejsze do odczytania, ponieważ na pierwszy rzut oka widzę, że jest to ustawienie domyślne false.

Również pomysł wymuszenia przesunięcia zawartości bloku w celu wstawienia nowej klauzuli jest podatny na błędy. Jest to w jakiś sposób związane z zasadą otwarcia / zamknięcia (kod powinien być otwarty dla rozszerzenia, ale zamknięty dla modyfikacji). Zmuszasz do modyfikacji istniejącego kodu (np. Koder może wprowadzić błąd w równoważeniu nawiasów klamrowych).

Wreszcie, doprowadzasz ludzi do odpowiedzi w określony sposób, który Twoim zdaniem jest właściwy. Przykładowo prezentujesz bardzo prosty przykład, a następnie stwierdzasz, że ludzie nie powinni go brać pod uwagę. Jednak prostota tego przykładu wpływa na całe pytanie:

  • Jeśli sprawa jest tak prosta jak prezentowana, moją odpowiedzią byłoby pominięcie jakiejkolwiek if else klauzul.

    return (sky.Color == Color.Blue);

  • Jeśli sprawa jest nieco bardziej skomplikowana, z różnymi klauzulami, odpowiedziałbym:

    bool CanLeaveWithoutUmbrella () {

    if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
        return true;
    } else if (sky.Humidity != Humidity.High) {
        return true;
    } else if (sky.Color == Color.Red) {
        return true;
    }
    
    return false;
    

    }

  • Jeśli sprawa jest skomplikowana i nie wszystkie klauzule zwracają wartość true (może zwracają wartość double), a ja chcę wprowadzić polecenie przerwania lub loggin, rozważę coś takiego:

    double CanLeaveWithoutUmbrella() {
    
        double risk = 0.0;
    
        if(sky.Color == Color.Blue && sky.Humidity == Humidity.High) {
            risk = 1.0;
        } else if (sky.Humidity != Humidity.High) {
            risk = 0.5;
        } else if (sky.Color == Color.Red) {
            risk = 0.8;
        }
    
        Log("value of risk:" + risk);
        return risk;
    

    }

  • Jeśli nie mówimy o metodzie, która coś zwraca, ale zamiast tego bloku if-else, który ustawia zmienną lub wywołuje metodę, to tak, dołączę ostatnią elseklauzulę.

Dlatego też należy wziąć pod uwagę prostotę przykładu.

FranMowinckel
źródło
Wydaje mi się, że popełniasz ten sam błąd, który popełnili inni, i trochę za bardzo zmieniasz przykład , analizując w ten sposób nowy problem, ale muszę poświęcić chwilę i zbadać proces dotarcia do twojego przykładu, więc na razie wygląda na to, że ważne dla mnie. I na marginesie, nie jest to związane z zasadą otwartego / zamkniętego . Zakres jest zbyt wąski, aby zastosować ten idiom. Warto jednak zauważyć, że jego zastosowanie na wyższym poziomie polegałoby na usunięciu całego aspektu problemu (poprzez wyeliminowanie potrzeby jakiejkolwiek modyfikacji funkcji).
Selali Adobor
1
Ale jeśli nie możemy zmodyfikować tego nadmiernie uproszczonego przykładu, dodając klauzule, ani zmodyfikować jego sposobu pisania, ani nie wprowadzać do niego żadnych zmian, to należy wziąć pod uwagę, że to pytanie dotyczy tylko tych dokładnie wierszy kodu i nie jest już pytaniem ogólnym. W takim przypadku masz całkowitą rację, nie dodaje ona żadnej złożoności (ani jej nie usuwa) z klauzuli else, ponieważ początkowo nie było złożoności. Jednak nie jesteś uczciwy, ponieważ sugerujesz pomysł dodania nowych klauzul.
FranMowinckel,
I na marginesie, nie twierdzę, że dotyczy to dobrze znanej zasady otwartego / zamkniętego. Wydaje mi się, że pomysł kierowania ludźmi do modyfikowania kodu zgodnie z Twoimi sugestiami jest podatny na błędy. Po prostu czerpię ten pomysł z tej zasady.
FranMowinckel,
Trzymaj się edycji, masz coś dobrego, piszę teraz komentarz
Selali Adobor
1

Chociaż opisany przypadek if-else ma większą złożoność, w większości (ale nie wszystkich) praktycznych sytuacjach nie ma to większego znaczenia.

Wiele lat temu, kiedy pracowałem nad oprogramowaniem używanym w przemyśle lotniczym (musiałem przejść proces certyfikacji), pojawiło się twoje pytanie. Okazało się, że rachunek „else” wiązał się z kosztami finansowymi, ponieważ zwiększał złożoność kodu.

Dlatego.

Obecność „else” stworzyła niejawny trzeci przypadek, który musiał zostać oceniony - ani „ani”, ani „else”. Może myślisz (tak jak wtedy) WTF?

Wyjaśnienie przebiegło w ten sposób ...

Z logicznego punktu widzenia istnieją tylko dwie opcje - „if” i „else”. Certyfikowaliśmy jednak plik obiektowy generowany przez kompilator. Dlatego, gdy istniało „inne”, trzeba było przeprowadzić analizę, aby potwierdzić, że wygenerowany kod rozgałęzienia był poprawny i że nie pojawiła się tajemnicza trzecia ścieżka.

Porównaj to z przypadkiem, w którym istnieje tylko „jeśli”, a nie „inne”. W takim przypadku istnieją tylko dwie opcje - ścieżka „jeśli” i ścieżka „nie-jeśli”. Dwa przypadki do oceny są mniej skomplikowane niż trzy.

Mam nadzieję że to pomoże.

iskrzący
źródło
czy w pliku obiektowym oba przypadki nie będą renderowane jako identyczne jmps?
Winston Ewert
@WinstonEwert - Można by oczekiwać, że będą takie same. Jednak to, co jest renderowane i czego się oczekuje, to dwie różne rzeczy, niezależnie od tego, jak bardzo się nakładają.
Sparky
(Chyba SE zjadł mój komentarz ...) To bardzo interesująca odpowiedź. Chociaż powiedziałbym, że ujawniony przypadek jest w pewnym sensie niszowy, pokazuje jednak, że niektóre narzędzia mogłyby znaleźć różnicę między nimi (nawet najbardziej powszechna metryka kodu oparta na przepływie, jaki widzę, cykliczność złożoności, nie zmierzyłaby żadnej różnicy.
Selali Adobor
1

Najważniejszym celem kodu jest bycie zrozumiałym jako oddany (w przeciwieństwie do łatwego refaktoryzacji, co jest przydatne, ale mniej ważne). Aby to zilustrować, można użyć nieco bardziej złożonego przykładu w języku Python:

def value(key):
    if key == FROB:
        return FOO
    elif key == NIK:
        return BAR
    [...]
    else:
        return BAZ

Jest to całkiem jasne - jest to odpowiednik casewyszukiwania instrukcji lub słownika, wersja wyższego poziomu return foo == bar:

KEY_VALUES = {FROB: FOO, NIK: BAR, [...]}
DEFAULT_VALUE = BAZ

def value(key):
    return KEY_VALUES.get(key, default=DEFAULT_VALUE)

Jest to wyraźniejsze, ponieważ chociaż liczba tokenów jest wyższa (32 vs 24 dla oryginalnego kodu z dwiema parami klucz / wartość), semantyka funkcji jest teraz wyraźna: To tylko wyszukiwanie.

(Ma to konsekwencje dla refaktoryzacji - jeśli chcesz mieć efekt uboczny, jeśli key == NIKmasz trzy możliwości:

  1. Powróć do stylu if/ elif/ elsei wstaw pojedynczą linię do key == NIKskrzynki.
  2. Zapisz wynik wyszukiwania do wartości i dodaj ifinstrukcję do valuespecjalnego przypadku przed zwróceniem.
  3. Umieść ifoświadczenie w dzwoniącym.

Teraz widzimy, dlaczego funkcja wyszukiwania jest potężnym uproszczeniem: sprawia, że ​​trzecia opcja jest oczywiście najprostszym rozwiązaniem, ponieważ oprócz powodowania mniejszej różnicy niż pierwsza opcja , jest jedyną, która zapewnia, że valuerobi tylko jedną rzecz. )

Wracając do kodu OP, myślę, że może to służyć jako przewodnik dla złożoności elseinstrukcji: kod z wyraźną elseinstrukcją jest obiektywnie bardziej złożony, ale ważniejsze jest to, czy sprawia, że semantyka kodu jest jasna i prosta. Na to pytanie należy odpowiedzieć indywidualnie, ponieważ każdy fragment kodu ma inne znaczenie.

l0b0
źródło
Podoba mi się twoja tabela odnośników przepisująca prostą obudowę przełącznika. I wiem, że to ulubiony idiom Pythona. W praktyce jednak często stwierdzam, że warunki warunkowe (aka caselub switchstwierdzenia) są mniej jednorodne. Kilka opcji to proste zwroty wartości, ale wtedy jedna lub dwie sprawy wymagają dodatkowego przetwarzania. A kiedy odkryjesz, że strategia wyszukiwania nie pasuje, musisz przepisać w zupełnie innej if elif... elseskładni.
Jonathan Eunice
Dlatego myślę, że wyszukiwanie zwykle powinno być liniowe lub funkcji, tak więc logika wokół wyniku wyszukiwania jest oddzielona od logiki wyszukiwania go w pierwszej kolejności.
l0b0
To pytanie pozwala spojrzeć na pytanie z wyższego poziomu, o którym zdecydowanie należy wspomnieć i pamiętać, ale czuję, że na
skrzynię nałożono
1

Myślę, że to zależy od tego, jakiego rodzaju ifoświadczenia używasz. Niektóre ifinstrukcje mogą być traktowane jako wyrażenia, a inne mogą być postrzegane wyłącznie jako instrukcje kontroli przepływu.

Twój przykład wygląda dla mnie jak wyrażenie. W językach podobnych do C operator trójskładnikowy ?:jest bardzo podobny do ifwyrażenia, ale jest wyrażeniem, a drugiej części nie można pominąć. Ma to sens, że gałęzi else nie można pominąć w wyrażeniu, ponieważ wyrażenie musi zawsze mieć wartość.

Za pomocą operatora potrójnego twój przykład wyglądałby tak:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue)
               ? true
               : false;
}

Ponieważ jest to wyrażenie boolowskie, naprawdę należy je uprościć do samego końca

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}

Zawarcie wyraźnej klauzuli else uczyni twoje zamiary wyraźniejszymi. Strażnicy mogą mieć sens w niektórych sytuacjach, ale przy wybieraniu między dwiema możliwymi wartościami, z których żadna nie jest wyjątkowa ani niezwykła, nie pomagają.

Michael Shaw
źródło
+1, OP to IMHO omawiające patologiczny przypadek, który powinien być lepiej napisany w sposób pokazany powyżej. O wiele częstszym przypadkiem „jeśli” z „powrotem” jest według mnie przypadek „straży”.
Doc Brown,
Nie wiem, dlaczego tak się dzieje, ale ludzie wciąż ignorują pierwszy akapit pytania. W rzeczywistości wszyscy używacie dokładnego wiersza kodu, który wyraźnie mówię, aby nie używać. I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.)
Selali Adobor,
I ponownie czytając swoje ostatnie pytanie, wydaje się, że nie rozumiesz jego intencji.
Selali Adobor,
Użyłeś przykładu, który praktycznie prosił o uproszczenie. Przeczytałem i zwróciłem uwagę na twój pierwszy akapit, dlatego mój pierwszy blok kodu w ogóle tam jest, zamiast przejść bezpośrednio do najlepszego sposobu na zrobienie tego konkretnego przykładu. Jeśli uważasz, że ja (lub ktokolwiek inny) źle zrozumiałem cel twojego pytania, być może powinieneś go edytować, aby wyrazić swoją wolę.
Michael Shaw
Zredagowałbym go, ale nie wiedziałbym, jak to wyjaśnić, ponieważ używam dokładnie wiersza kodu, który piszesz i mówisz: „nie rób tego”. Istnieją nieskończone sposoby na przepisanie kodu i usunięcie pytania, nie mogę ich wszystkich
opisać
1

Kolejną rzeczą do przemyślenia w tym argumencie są nawyki, które promuje każde podejście.

  • if / else zmienia próbkę kodowania w kategoriach gałęzi. Jak mówisz, czasami ta wyraźność jest dobra. Istnieją naprawdę dwie możliwe ścieżki wykonania i chcemy to podkreślić.

  • W innych przypadkach istnieje tylko jedna ścieżka wykonania, a następnie wszystkie te wyjątkowe rzeczy, o które programiści muszą się martwić. Jak wspomniałeś, klauzule ochronne są do tego świetne.

  • Oczywiście jest 3 lub więcej przypadków (n), w których zwykle zachęcamy do porzucenia łańcucha if / else i użycia instrukcji case / switch lub polimorfizmu.

Główną zasadą, o której tu pamiętam, jest to, że metoda lub funkcja powinna mieć tylko jeden cel. Każdy kod z instrukcją if / else lub switch służy wyłącznie do rozgałęzienia. Powinien więc być absurdalnie krótki (4 linie) i delegować tylko do właściwej metody lub dawać oczywisty wynik (jak twój przykład). Kiedy twój kod jest tak krótki, trudno przeoczyć te wielokrotne zwroty :) Podobnie jak „goto uważany za szkodliwy”, każde oświadczenie rozgałęziające może być nadużywane, więc warto poświęcić trochę krytycznej przemyślenia na inne stwierdzenia. Jak wspomniałeś, sam blok else zapewnia miejsce do zlepiania się kodu. Ty i twój zespół będziecie musieli zdecydować, co się z tym czuje.

To, na co tak naprawdę narzeka to narzędzie, to fakt, że masz zwrot / przerwę / rzut w bloku if. Jeśli o to chodzi, napisałeś klauzulę ochronną, ale spieprzyłeś ją. Możesz uszczęśliwić narzędzie lub „zabawić” jego sugestię bez jej akceptowania.

Steve Jackson
źródło
Nigdy nie myślałem o tym, że narzędzie może rozważać klauzulę ochronną, gdy tylko pasuje do wzorca jednego, prawdopodobnie tak jest i wyjaśnia to uzasadnienie jednej „grupy zwolenników” z powodu jej braku
Selali Adobor
@AssortedTrailmix Racja, myślisz elsesemantycznie, narzędzia do analizy kodu zwykle szukają obcych instrukcji, ponieważ często uwidaczniają nieporozumienie dotyczące kontroli. elsePo dokonaniu ifże powraca marnuje bitów. if(1 == 1)często wyzwala ten sam rodzaj ostrzeżenia.
Steve Jackson,
1

Myślę, że odpowiedź brzmi: to zależy. Twój przykładowy kod (jak pośrednio wskazujesz) po prostu zwraca ocenę warunku i najlepiej jest go przedstawić, jak wiadomo, jako pojedyncze wyrażenie.

Aby ustalić, czy dodanie warunku else wyjaśnia lub ukrywa kod, musisz ustalić, co reprezentuje IF / ELSE. Czy jest to (jak w twoim przykładzie) wyrażenie? Jeśli tak, to wyrażenie należy wyodrębnić i użyć. Czy to warunek ochronny? Jeśli tak, to ELSE jest niepotrzebne i wprowadza w błąd, ponieważ sprawia, że ​​obie gałęzie wydają się równoważne. Czy to przykład polimorfizmu proceduralnego? Wyodrębnij to. Czy to stwarza warunki wstępne? To może być niezbędne.

Zanim zdecydujesz się na włączenie lub wyeliminowanie ELSE, zdecyduj, co reprezentuje, co powie ci, czy powinien on być obecny, czy nie.

jmoreno
źródło
0

Odpowiedź jest nieco subiektywna. elseBlok może pomóc czytelności poprzez ustalenie, że jeden blok wykonaniem kodu wyłącznie do innego bloku kodu, bez konieczności przeczytać obu bloków. Może to być pomocne, jeśli, powiedzmy, oba bloki są stosunkowo długie. Są przypadki, w których posiadanie ifs bez elses może być bardziej czytelne. Porównaj następujący kod z wieloma if/elseblokami:

if(a == b) {
    if(c <= d) {
        if(e != f) {
            a.doSomeStuff();
            c.makeSomeThingsWith(b);
            f.undo(d, e);
            return 9;
        } else {
            e++;
            return 3;
        }
    } else {
        return 1;
    }
} else {
    return 0;
}

z:

if(a != b) {
    return 0;
}

if(c > d) {
    return 1;
}

if(e != f) {
    e++;
    return 3;
}

a.doSomeStuff();
c.makeSomeThingsWith(b);
f.undo(d, e);
return 9;

Drugi blok kodu zmienia zagnieżdżone ifinstrukcje w klauzule zabezpieczające. Zmniejsza to wcięcia i sprawia, że ​​niektóre warunki powrotu są wyraźniejsze („jeśli a != b, to wrócimy 0!”)


W komentarzach wskazano, że w moim przykładzie mogą być pewne dziury, więc jeszcze trochę go dopracuję.

Mogliśmy napisać tutaj pierwszy blok kodu, aby był bardziej czytelny:

if(a != b) {
    return 0;
} else if(c > d) {
    return 1;
} else if(e != f) {
    e++;
    return 3;
} else {
    a.doSomeStuff();
    c.makeSomeThingsWith(b);
    f.undo(d, e);
    return 9;
}

Ten kod jest równoważny obu powyższym blokom, ale przedstawia pewne wyzwania. elseKlauzula tutaj łączy te if razem. W powyższej wersji klauzuli ochronnej klauzule ochronne są od siebie niezależne. Dodanie nowego oznacza po prostu napisanie nowego ifmiędzy ostatnią ifa resztą logiki. Można to również zrobić przy niewielkim obciążeniu poznawczym. Różnica polega jednak na tym, że przed tą nową klauzulą ​​ochronną musi wystąpić pewna dodatkowa logika. Gdy oświadczenia były niezależne, moglibyśmy:

...
if(e != f) {
    e++;
    return 3;
}

g.doSomeLogicWith(a);

if(!g.isGood()) {
    return 4;
}

a.doSomeStuff();
...

Z połączonym if/elsełańcuchem nie jest to takie łatwe. W rzeczywistości najłatwiejszą drogą jest zerwanie łańcucha, aby bardziej przypominać wersję klauzuli ochronnej.

Ale to naprawdę ma sens. Używanie if/elsesłabo implikuje związek między częściami. Możemy przypuszczać, że a != bjest to w jakiś sposób związane c > dz if/elsewersją łańcuchową. Takie przypuszczenie byłoby mylące, ponieważ z wersji klauzuli wartowniczej wynika, że ​​w rzeczywistości nie są one powiązane. Oznacza to, że możemy zrobić więcej dzięki niezależnym klauzulom, takim jak zmiana ich kolejności (być może w celu optymalizacji wydajności zapytań lub usprawnienia logicznego przepływu). Możemy też je poznawczo zignorować, gdy miniemy ich. W if/elsełańcuchu nie jestem pewien, czy relacja równoważności pomiędzy ai bnie pojawi się później.

Związek sugerowany przez elsejest również widoczny w głęboko zagnieżdżonym przykładowym kodzie, ale w inny sposób. Te elseinstrukcje są oddzielone od uzależnione są przyłączone, i są one podobne w odwrotnej kolejności do warunkowych (pierwszy elsejest dołączony do ostatniego ifostatni elsejest przymocowany do pierwszego if). Powoduje to dysonans poznawczy, w którym musimy cofać się przez kod, próbując wyrównać wcięcia w naszych mózgach. To trochę tak, jakby próbować dopasować kilka nawiasów. Jest jeszcze gorzej, jeśli wcięcie jest błędne. Opcjonalne nawiasy klamrowe też nie pomagają.

Zapomniałem wspomnieć o przypadku, w którym zgadzam się z pominięciem bloku else i jest to, gdy używam bloku if do zastosowania ograniczenia, które musi być logicznie spełnione dla całego następującego kodu, takiego jak kontrola zerowa (lub inne zabezpieczenia ).

To niezła koncesja, ale tak naprawdę nie unieważnia żadnej odpowiedzi. Mogę powiedzieć, że w twoim przykładzie kodu:

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color != Color.Blue)
    {
        return false;
    }
    return true;
}

poprawną interpretacją takiego kodu może być to, że „musimy wyjść z parasolką, jeśli niebo nie jest niebieskie”. Istnieje ograniczenie polegające na tym, że niebo musi być niebieskie, aby poniższy kod działał poprawnie. Spełniłem definicję twojej koncesji.

Co jeśli powiem, że jeśli kolor nieba jest czarny, to jest czysta noc, więc parasol nie jest konieczny. (Istnieją prostsze sposoby na napisanie tego, ale są prostsze sposoby na napisanie całego przykładu).

bool CanLeaveWithoutUmbrella()
{
    if(sky.Color == Color.Blue)
    {
        return true;
    }

    if(sky.Color == Color.Black)
    {
        return true;
    }

    return false;
}

Tak jak w większym przykładzie powyżej, mogę po prostu dodać nową klauzulę bez potrzeby. W if/else, nie może być więc pewien, że nie będzie bałagan coś w górę, zwłaszcza jeśli logika jest bardziej skomplikowana.

Zwrócę też uwagę, że twój prosty przykład również nie jest taki prosty. Test na wartość niebinarną nie jest taki sam jak odwrotność tego testu z odwróconymi wynikami.

Cbojar
źródło
1
Moja edycja pytania dotyczy tej sprawy. Kiedy ograniczenie, które musi być logicznie spełnione dla całego następującego kodu, takie jak kontrola zerowa (lub wszelkie inne zabezpieczenia), zgadzam się, że pominięcie elsebloku jest niezbędne dla czytelności.
Selali Adobor
1
Twoja pierwsza wersja jest trudna do uchwycenia, ale nie sądzę, że jest to konieczne przy użyciu if / else. Zobacz, jak bym to napisał: gist.github.com/winstonewert/c9e0955bc22ce44930fb .
Winston Ewert
@WinstonEwert Zobacz zmiany odpowiadające na twoje komentarze.
cbojar
Twoja edycja zawiera kilka ważnych punktów. I z pewnością uważam, że klauzule ochronne mają swoje miejsce. Ale w ogólnym przypadku jestem za innymi klauzulami.
Winston Ewert
0

Za bardzo uprościłeś swój przykład. Każda alternatywa może być bardziej czytelna, w zależności od większej sytuacji: w szczególności zależy to od tego, czy jedna z dwóch gałęzi stanu jest nienormalna . Pokażę ci: w przypadku, gdy jedna z gałęzi jest równie prawdopodobna,

void DoOneOfTwoThings(bool which)
{
    if (which)
        DoThingOne();
    else
        DoThingTwo();
}

... jest bardziej czytelny niż ...

void DoOneOfTwoThings(bool which)
{
    if (which) {
        DoThingOne();
        return;
    }
    DoThingTwo();
}

... ponieważ pierwszy ma równoległą strukturę, a drugi nie. Ale kiedy większość funkcji jest poświęcona jednemu zadaniu i musisz najpierw sprawdzić niektóre warunki wstępne, ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input)) {
        ProcessInputOfTypeTwo(input);
        return;
    }
    ProcessInputDefinitelyOfTypeOne(input);

}

... jest bardziej czytelny niż ...

void ProcessInputOfTypeOne(String input)
{
    if (InputIsReallyTypeTwo(input))
        ProcessInputOfTypeTwo(input);
    else
        ProcessInputDefinitelyOfTypeOne(input);
}

... ponieważ celowe nie konfigurowanie równoległej struktury stanowi wskazówkę dla przyszłego czytelnika, że ​​uzyskiwanie danych wejściowych „naprawdę typu drugiego” tutaj jest nienormalne . (W prawdziwym życiu ProcessInputDefinitelyOfTypeOnenie byłaby to osobna funkcja, więc różnica byłaby jeszcze bardziej wyraźna.)

zwol
źródło
Lepiej byłoby wybrać bardziej konkretny przykład dla drugiego przypadku. Moja natychmiastowa reakcja na to brzmi: „to nie może być tak nienormalne, jeśli i tak je przetworzysz”.
Winston Ewert
Nieprawidłowy termin @WinstonEwert jest prawdopodobnie niewłaściwym terminem. Zgadzam się jednak z Zackiem, że jeśli masz wartość domyślną (przypadek 1) i jeden wyjątek, należy ją zapisać jako » if-> Wykonaj wyjątek i pozostaw« jako strategię wczesnego powrotu . Pomyśl o kodzie, w którym domyślnie jest więcej sprawdzeń, szybciej byłoby zająć się przypadkiem wyjątkowym.
Thomas Junk
Wydaje się, że to pasuje do przypadku, z którym rozmawiałem when using an if block to apply a constraint that must be logically satisfied for all following code, such as a null-check (or any other guards). Logicznie, każda praca ProcessInputOfTypeOnezależy od tego, że !InputIsReallyTypeTwo(input)musimy ją wychwycić poza naszym normalnym przetwarzaniem. Normalnie ProcessInputOfTypeTwo(input);byłoby, throw ICan'tProccessThatExceptionco uczyniłoby to podobieństwo bardziej oczywistym.
Selali Adobor
@WinstonEwert Mógłbym wyrazić to lepiej, tak. Myślałem o sytuacji, która często pojawia się podczas ręcznego kodowania tokenizatorów: na przykład w CSS sekwencja znaków „ u+zwykle wprowadza token „zakresu Unicode”, ale jeśli następny znak nie jest cyfrą szesnastkową, następnie należy wykonać kopię zapasową i przetworzyć „u” jako identyfikator. Tak więc główna pętla skanera wysyła polecenie „zeskanować token zakresu Unicode” nieco nieprecyzyjnie i zaczyna się od „... jeśli jesteśmy w tym niezwykłym szczególnym przypadku, wykonaj kopię zapasową, a następnie wywołaj podprogram skanowania-identyfikatora”.
zwolnić
@ AssortedTrailmix Nawet jeśli przykład, o którym myślałem, nie pochodzi ze starszej bazy kodu, w której wyjątki nie mogą być użyte, nie jest to warunek błędu , tylko kod wywołujący ProcessInputOfTypeOneużywa nieprecyzyjnego, ale szybkiego sprawdzania, które czasami zamiast tego łapie typ dwa.
zwolnić
0

Tak, występuje wzrost złożoności, ponieważ klauzula else jest zbędna . Dlatego lepiej pomijać kod, aby zachować jego oszczędność i wredność. Działa tak samo, jak pomijanie nadmiarowych thiskwalifikatorów (Java, C ++, C #) i pomijanie nawiasów w returninstrukcjach i tak dalej.

Dobry programista myśli „co mogę dodać?” podczas gdy świetny programista myśli „co mogę usunąć?”.

vidstige
źródło
1
Kiedy widzę pominięcie elsebloku, jeśli jest to wyjaśnione w jednej linijce (co nie jest często), często ma tego rodzaju rozumowanie, więc widzę +1 za przedstawienie „domyślnego” argumentu, ale nie okaże się, że jest to wystarczająco mocne uzasadnienie. A good programmer things "what can I add?" while a great programmer things "what can I remove?".Wydaje się, że jest to popularne powiedzenie, że wiele osób przesadza bez dokładnego rozważenia, a ja osobiście uważam, że jest to jeden z takich przypadków.
Selali Adobor
Tak, czytając twoje pytanie od razu poczułem, że już podjąłeś decyzję, ale chciałem potwierdzenia. Ta odpowiedź oczywiście nie jest tym, czego chciałeś, ale czasem ważne jest, aby wziąć pod uwagę opinie, z którymi się nie zgadzasz. Może coś też jest?
vidstige
-1

Zauważyłem, zmieniając zdanie w tej sprawie.

Kiedy zadałem to pytanie rok temu, odpowiedziałbym na coś takiego:

»Powinien być tylko jeden entryexit metodzie i jeden «. Mając to na uwadze, sugerowałbym przefakturowanie kodu w celu:

bool CanLeaveWithoutUmbrella()
{
    bool result

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    else
    {
        result = false;
    }

    return result;
}

Z punktu widzenia komunikacji jasne jest , co należy zrobić. Ifcoś jest w tym przypadku, manipuluj wynikiem w jeden sposób , else manipuluj nim w inny sposób .

Ale wiąże się to z niepotrzebnym else . elseWyraża default-przypadek . Tak więc w drugim kroku mógłbym to zmienić

bool CanLeaveWithoutUmbrella()
{
    bool result=false

    if(sky.Color == Color.Blue)
    {
        result = true;
    }
    return result;
}

Następnie pojawia się pytanie: po co w ogóle używać zmiennej tymczasowej? Kolejny etap refaktoryzacji prowadzi do:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue)
    {
        return true;
    }
    return false;
}

To jasne w tym, co robi. Idę nawet o krok dalej:

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) return true;
    return false;
}

Lub dla osób o słabych nerwach :

bool CanLeaveWithoutUmbrella()
{

    if(sky.Color == Color.Blue) { return true; }
    return false;
}

Można go odczytać tak: »CanLeaveWithoutUmbrella zwraca bool . Jeśli nic specjalnego się nie wydarzy, zwraca false «To jest pierwsze czytanie, od góry do dołu.

A potem „analizujesz” warunek »Jeśli warunek jest spełniony, zwraca wartość true « Możesz całkowicie pominąć warunek i zrozumieć, co ta funkcja robi w domyślnych przypadku. Twoje oko i umysł muszą jedynie przeglądać niektóre litery po lewej stronie, bez czytania części po prawej stronie.

Mam wrażenie, że reszta bardziej bezpośrednio określa intencję, stwierdzając, że kod w obu blokach jest bezpośrednio powiązany.

Tak to prawda. Ale ta informacja jest zbędna . Masz domyślny przypadek i jeden „wyjątek”, który jest objęty if-statement.

Kiedy mam do czynienia ze skomplikowanymi strukturami, wybrałbym inną strategię, pomijając w ifogóle brzydkiego brata switch.

Thomas Junk
źródło
Redundancja +1 zdecydowanie pokazuje wzrost nie tylko złożoności, ale i zamieszania. Wiem, że zawsze musiałem dwukrotnie sprawdzać kod napisany w ten sposób właśnie z powodu nadmiarowości.
dietbuddha,
1
Czy możesz usunąć skrzynki po rozpoczęciu skrzynki So this is clear in what it does...i ją rozwinąć? (Wcześniejsze przypadki mają również wątpliwe znaczenie). Mówię to, ponieważ to pytanie zadaje nam pytanie. Podałeś swoją postawę, pomijając blok „else”, i faktycznie jest to pozycja, która wymaga więcej wyjaśnień (i tej, która skłoniła mnie do zadania tego pytania). Z pytania:I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.)
Selali Adobor
@AssortedTrailmix Sure! Co dokładnie powinienem rozwinąć? Ponieważ początkowe pytanie brzmiało: „Czy inne bloki zwiększają złożoność kodu?” I moja odpowiedź brzmi: tak. W takim przypadku można to pominąć.
Thomas Junk
I nie! Nie rozumiem opinii negatywnej.
Thomas Junk
-1

Krótko mówiąc, w tym przypadku pozbywanie się elsesłowa kluczowego jest dobrą rzeczą. Jest tutaj całkowicie redundantny i w zależności od tego, w jaki sposób sformatujesz kod, doda od jednego do trzech zupełnie bezużytecznych wierszy. Zastanów się, co jest w ifbloku:

return true;

Dlatego jeśli ifblok miałby zostać wprowadzony, cała reszta funkcji z definicji nie jest wykonywana. Ale jeśli nie zostanie wprowadzone, ponieważ nie ma dalszych ifinstrukcji, cała reszta funkcji zostanie wykonana. Byłoby zupełnie inaczej, gdyby returninstrukcja nie znajdowała się w ifbloku, w którym to przypadku obecność lub brak elsebloku miałby wpływ, ale tutaj nie miałby wpływu.

W swoim pytaniu, po odwróceniu ifstanu i pominięciu else, stwierdziłeś:

Ktoś może teraz dodać nowy blok if na podstawie warunku po pierwszym przykładzie bez natychmiastowego prawidłowego rozpoznania pierwszego warunku nakłada ograniczenie na swój własny warunek.

Muszą wtedy przeczytać kod nieco dokładniej. Używamy języków wysokiego poziomu i przejrzystej organizacji kodu, aby złagodzić te problemy, ale dodanie całkowicie redundantnego elsebloku dodaje do kodu „szum” i „bałagan” i nie powinniśmy również musieć odwracać ani przywracać naszych ifwarunków. Jeśli nie potrafią odróżnić, nie wiedzą, co robią. Jeśli potrafią rozpoznać różnicę, zawsze mogą sami odwrócić pierwotne ifwarunki.

Zapomniałem wspomnieć o przypadku, w którym zgadzam się z pominięciem bloku else i jest to, gdy używam bloku if do zastosowania ograniczenia, które musi być logicznie spełnione dla całego następującego kodu, takiego jak kontrola zerowa (lub inne zabezpieczenia ).

Tak się jednak zasadniczo dzieje. Wracasz z ifbloku, więc ifocena warunku do false jest ograniczeniem, które musi być logicznie spełnione dla całego następującego kodu.

Czy wprowadzono wzrost złożoności, nie uwzględniając bloku else?

Nie, będzie to oznaczać zmniejszenie złożoności kodu, a nawet zmniejszenie skompilowanego kodu, w zależności od tego, czy zostaną przeprowadzone pewne supertrywialne optymalizacje.

Panzercrisis
źródło
2
„Muszą przeczytać kod nieco dokładniej”. Istnieje styl kodowania, dzięki czemu programiści mogą mniej zwracać uwagę na drobne szczegóły i więcej uwagi na to, co robią. Jeśli twój styl powoduje, że niepotrzebnie zwracasz uwagę na te szczegóły, jest to zły styl. „... całkowicie bezużyteczne linie ...” Nie są bezużyteczne - pozwalają na pierwszy rzut oka zobaczyć, czym jest struktura, bez marnowania czasu na zastanawianie się, co by się stało, gdyby ta część lub ta część została wykonana.
Michael Shaw,
@MichaelShaw Myślę, że odpowiedź Aviv Cohn dotyczy tego, o czym mówię.
Panzercrisis,
-2

W konkretnym podanym przykładzie tak jest.

  • Zmusza recenzenta do ponownego przeczytania kodu, aby upewnić się, że nie jest to błąd.
  • Dodaje (niepotrzebną) złożoność cykliczną, ponieważ dodaje jeden węzeł do wykresu przepływu.

Myślę, że nie może być prostsze, że:

bool CanLeaveWithoutUmbrella()
{
    return (sky.Color == Color.Blue);
}
Tulains Córdova
źródło
6
W szczególności odnoszę się do faktu, że ten bardzo uproszczony przykład można przepisać w celu usunięcia ifstwierdzenia. W rzeczywistości wyraźnie odradzam dalsze uproszczenie, używając dokładnie takiego kodu, jakiego użyłeś. Dodatkowo elseblok nie zwiększa złożoności cyklicznej .
Selali Adobor
-4

Zawsze staram się pisać mój kod, aby intencja była jak najbardziej jasna. W twoim przykładzie brakuje kontekstu, więc zmodyfikuję go trochę:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        else
        {
            return false;
        }
    }
}

Wydaje się, że naszą intencją jest to, że możemy wyjść bez parasola, gdy niebo jest niebieskie. Jednak ta prosta intencja zaginęła w morzu kodu. Istnieje kilka sposobów, dzięki którym możemy łatwiej to zrozumieć.

Po pierwsze, masz pytanie dotyczące elseoddziału. Tak czy inaczej, nie mam nic przeciwko, ale zwykle pomijam else. Rozumiem argument o byciu jawnym, ale moim zdaniem ifstwierdzenia powinny zawierać „opcjonalne” gałęzie, takie jak ta return true. Nie nazwałbym tego return falseoddziału „opcjonalnym”, ponieważ działa on jako nasz domyślny. Tak czy inaczej, uważam to za bardzo drobny problem i o wiele mniej ważny niż następujące:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        if(this.color == Color.Blue)
        {
            return true;
        }
        return false;
    }
}

O wiele ważniejsze jest to, że twoje oddziały robią za dużo! Gałęzie, jak wszystko, powinny być „tak proste, jak to możliwe, ale nie więcej”. W tym przypadku użyłeś ifinstrukcji, która rozgałęzia się między blokami kodu (kolekcjami instrukcji), kiedy wszystko, czego naprawdę potrzebujesz, to wyrażenia (wartości). Jest to jasne, ponieważ a) bloki kodu zawierają tylko pojedyncze instrukcje oraz b) są to te same instrukcje ( return). Możemy użyć operatora trójskładnikowego, ?:aby zawęzić gałąź do innej części:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue)
            ? true
            : false;
    }
}

Teraz dochodzimy do oczywistej redundancji, z którą wynik jest identyczny this.color == Color.Blue. Wiem, że twoje pytanie prosi nas o zignorowanie tego, ale myślę, że naprawdę ważne jest, aby zwrócić uwagę, że jest to bardzo proceduralny sposób myślenia pierwszego rzędu: rozkładasz wartości wejściowe i konstruujesz niektóre wartości wyjściowe. Zgadza się, ale jeśli to możliwe, znacznie silniej jest myśleć w kategoriach relacji lub transformacji między wejściem a wyjściem. W tym przypadku związek jest oczywiście taki sam, ale często widzę taki zawiły kod proceduralny, który przesłania pewne proste relacje. Przejdźmy do uproszczenia:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool CanLeaveWithoutUmbrella()
    {
        return (this.color == Color.Blue);
    }
}

Następną rzeczą, na którą zwracam uwagę, jest to, że twój interfejs API zawiera podwójnie negatywne: możliwe, CanLeaveWithoutUmbrellaże tak false. To oczywiście upraszcza NeedUmbrellabycie true, więc zróbmy to:

class Sky {
    Sky(Colour c)
    {
        this.color = c;
    }
    bool NeedUmbrella()
    {
        return (this.color != Color.Blue);
    }
}

Teraz możemy zacząć myśleć o twoim stylu kodowania. Wygląda na to, że używasz języka zorientowanego obiektowo, ale używając ifinstrukcji do rozgałęziania się między blokami kodu, skończyło się na pisaniu procedur kodu .

W kodzie zorientowanym obiektowo wszystkie bloki kodu są metodami, a wszystkie rozgałęzienia są wykonywane za pomocą dynamicznej wysyłki , np. za pomocą klas lub prototypów. Można się spierać, czy to upraszcza sprawę, ale powinno to uczynić kod bardziej spójnym z resztą języka:

class Sky {
    bool NeedUmbrella()
    {
        return true;
    }
}

class BlueSky extends Sky {
    bool NeedUmbrella()
    {
        return false;
    }
}

Zauważ, że użycie trójskładnikowych operatorów do rozgałęzienia między wyrażeniami jest powszechne w programowaniu funkcjonalnym .

Warbo
źródło
Pierwszy akapit odpowiedzi wydaje się być na dobrej drodze do udzielenia odpowiedzi na pytanie, ale natychmiast przechodzi w dokładny temat, który wyraźnie zadaję, ignorując ten bardzo prosty przykład . Z pytaniem: I ask that you ignore the many other ways the function can be written, and changes that can be made to it. (I know most people are itching to write return sky.Color == Color.Blue.). W rzeczywistości używasz dokładnej linii kodu, o której wspomniałem. Ponadto, chociaż ta część pytania jest nie na temat, powiedziałbym, że posuwasz się za daleko w swoich wnioskach. Musisz radykalnie zmienił co kod jest.
Selali Adobor
@AssortedTrailmix To jest wyłącznie kwestia stylu. Sensowne jest dbanie o styl i ignorowanie go (koncentrowanie się tylko na wynikach). Nie sądzę, że warto dbać o return false;vs else { return false; }, nie dbając o polaryzację gałęzi, dynamiczne wysyłanie, trójskładniki itp. Jeśli piszesz kod proceduralny w języku OO, konieczna jest radykalna zmiana;)
Warbo
Możesz powiedzieć coś takiego w ogólności, ale nie ma to zastosowania, gdy odpowiadasz na pytanie z dobrze zdefiniowanymi parametrami (szczególnie, gdy nie przestrzegając tych parametrów, możesz „wyodrębnić” całe pytanie). Powiedziałbym również, że ostatnie zdanie jest po prostu błędne. OOP dotyczy „abstrakcji proceduralnej”, a nie „proceduralnego zatarcia”. Powiedziałbym, że fakt przejścia od jednego liniowca do nieskończonej liczby klas (jedna na każdy kolor) pokazuje, dlaczego. Dodatkowo wciąż zastanawiam się, co dynamiczna wysyłka powinna mieć wspólnego z if/elseinstrukcją i jaka jest biegunowość gałęzi.
Selali Adobor
Nigdy nie mówiłem, że rozwiązanie OO jest lepsze (w rzeczywistości wolę programowanie funkcjonalne), po prostu powiedziałem, że jest bardziej spójne z językiem. Przez „polaryzację” rozumiałem, która rzecz jest „prawdziwa”, a która „fałszywa” (zamieniłem ją na „NeedUmbrella”). W projekcie OO cały przepływ sterowania jest determinowany przez dynamiczną wysyłkę. Na przykład Smalltalk nie zezwala na nic innego en.wikipedia.org/wiki/Smalltalk#Control_structures (patrz także c2.com/cgi/wiki?HeInventedTheTerm )
Warbo
2
Głosowałbym za tym, dopóki nie przejdę do ostatniego akapitu (o OO), który zamiast tego przyjął opinię negatywną! To jest właśnie rodzaj bezmyślnego nadużywania konstrukcji OO, które produkują kod ravioli , który jest tak samo nieczytelny jak spaghetti.
zwolnić