Deweloper twierdzi, że instrukcje nie powinny mieć zanegowanych warunków i zawsze powinny mieć inny blok

169

Mam znajomego, bardziej doświadczonego programistę niż ja. Rozmawialiśmy o praktykach programistycznych i zaskoczyło mnie jego podejście do wypowiedzi „jeśli”. Nalega na pewne praktyki dotyczące stwierdzeń, które uważam za dość dziwne.

Po pierwsze , po instrukcji if powinna następować instrukcja else, niezależnie od tego, czy jest coś do wprowadzenia, czy nie. Co prowadzi do kodu wyglądającego tak:

if(condition) 
{
    doStuff();
    return whatever;
}
else
{
}

Po drugie , lepiej jest sprawdzić prawdziwe wartości niż fałsz. Oznacza to, że lepiej jest przetestować zmienną „doorClosed” zamiast zmiennej „! DoorOpened”

Jego argumentem jest to, że sprawia, że jaśniejsze co kod robi.

Co mnie trochę dezorientuje, ponieważ połączenie tych dwóch reguł może doprowadzić go do napisania tego rodzaju kodu, jeśli chce coś zrobić, gdy warunek nie jest spełniony.

if(condition)
{
}
else
{
    doStuff();
    return whatever;
}

Mam wrażenie, że jest to naprawdę brzydkie i / lub poprawa jakości, jeśli w ogóle, jest znikoma. Ale jako junior jestem skłonny wątpić w mój instynkt.

Więc moje pytania brzmią: czy jest to dobra / zła / „nieważna” praktyka? Czy to powszechna praktyka?

Patsuan
źródło
57
Czy to możliwe, że twój współpracownik pochodzi ze środowiska, w którym zagrożone jest życie i kończyna? Wydaje mi się, że widziałem kilka wzmianek o takich rzeczach w systemach, w których mnóstwo bada testowania i automatycznej analizy kodu i gdzie popełnienie błędu może dosłownie kosztować kogoś życie.
jpmc26,
11
Co mówi przewodnik po kodzie firmy?
Thorbjørn Ravn Andersen
4
Częściowa odpowiedź na twoje pytanie „drugie”. Jeśli jeden z bloków akcji jest długi, a drugi krótki, przetestuj warunek, który stawia blok pierwszy na pierwszym miejscu, tak aby mniej prawdopodobne było pominięcie go podczas czytania kodu. Ten warunek może być zaprzeczeniem lub przemianowaniem, aby uczynić go sprawdzianem prawdziwości.
Ethan Bolker,
9
Resharper miałby coś do powiedzenia twojemu przyjacielowi, podobnie jak „Twój kod jest zbędny i bezużyteczny, czy chcesz, żebym go usunął / przefiltrował?”
BgrWorker

Odpowiedzi:

184

Jawny elseblok

Pierwsza reguła po prostu zanieczyszcza kod i sprawia, że ​​nie jest on ani bardziej czytelny, ani mniej podatny na błędy. Przypuszczam, że celem twojego kolegi jest wyraźne, pokazując, że programista był w pełni świadomy, że warunek może się ocenić false. Chociaż dobrze jest być jawnym, taka jawność nie powinna kosztować trzech dodatkowych wierszy kodu .

Nie wspominam nawet o tym, że po ifoświadczeniu niekoniecznie musi elseznajdować się „ albo” albo „nic”: może też następować jeden lub więcej elif.

Obecność returnoświadczenia pogarsza sytuację. Nawet jeśli faktycznie masz kod do wykonania w elsebloku, bardziej zrozumiałe będzie wykonanie tego w następujący sposób:

if (something)
{
    doStuff();
    return whatever;
}

doOtherThings();
return somethingElse;

To sprawia, że ​​kod zajmuje dwa wiersze mniej, i odradza elseblok. Chodzi o klauzule ochronne .

Zauważ jednak, że technika twojego kolegi może częściowo rozwiązać bardzo nieprzyjemny wzór ułożonych bloków warunkowych bez spacji:

if (something)
{
}
if (other)
{
}
else
{
}

W poprzednim kodzie brak rozsądnego podziału linii po pierwszym ifbloku bardzo łatwo źle interpretuje kod. Jednak chociaż reguła twojego kolegi utrudniłaby błędne odczytanie kodu, łatwiejszym rozwiązaniem byłoby po prostu dodanie nowego wiersza.

Testuj true, a niefalse

Druga reguła może mieć jakiś sens, ale nie w obecnej formie.

Nie jest prawdą, że testowanie drzwi zamkniętych jest bardziej intuicyjne niż testowanie drzwi nieotwartych . Negacje, a zwłaszcza negacje zagnieżdżone, są zwykle trudne do zrozumienia:

if (!this.IsMaster || (!this.Ready && !this.CanWrite))

Aby rozwiązać ten problem, zamiast dodawać puste bloki, utwórz dodatkowe właściwości, w stosownych przypadkach, lub zmienne lokalne .

Powyższy warunek można dość łatwo odczytać:

if (this.IsSlave || (this.Synchronizing && this.ReadOnly))
Arseni Mourzenko
źródło
169
Puste bloki zawsze wyglądają dla mnie jak błędy. Natychmiast zwróci moją uwagę i zapytam: „Dlaczego to tu jest?”
Nelson
50
@ Nee Negowanie warunku niekoniecznie wymaga dodatkowego czasu procesora. C kompilowany do x86 może po prostu zamienić instrukcji skoku z JZ(Przełącz jeśli zero) if (foo)do JNZ(Przełącz jeśli nie zero) if (!foo).
8bittree
71
utwórz dodatkowe właściwości z wyraźnymi nazwami ”: chyba że masz głębszą analizę domeny, może to zmienić globalny zakres (oryginalna klasa) w celu rozwiązania lokalnego problemu (zastosowanie określonej reguły biznesowej). Jedną z rzeczy, które można tutaj zrobić, jest utworzenie lokalnych booleanów z pożądanym stanem, takich jak „var isSlave =! This.IsMaster” i zastosowanie opcji if z lokalnymi booleanami zamiast tworzenia kilku innych właściwości. Więc posłuchaj porady z odrobiną soli i poświęć trochę czasu na analizę domeny przed utworzeniem nowych właściwości.
Machado
82
Nie chodzi o „trzy wiersze kodu”, chodzi o pisanie dla odpowiednich odbiorców (kogoś, kto ma przynajmniej podstawową znajomość programowania). ifJest już wyraźnie o tym, że stan może być fałszywe, albo nie będzie testowanie dla niego. Pusty blok sprawia, że ​​wszystko jest mniej jasne, a nie więcej, ponieważ żaden czytelnik nie jest przygotowany, aby się tego spodziewać, a teraz muszą się zatrzymać i zastanowić, jak mógł się tam dostać.
hobbs
15
@Zaznacz to mniej jasne, nawet z komentarzem, niż mówienie if (!commonCase) { handle the uncommon case },.
Paul,
62

Jeśli chodzi o pierwszą zasadę, jest to przykład bezużytecznego pisania. Pisanie nie tylko zajmuje więcej czasu, ale powoduje ogromne zamieszanie dla osób czytających kod. Jeśli kod nie jest potrzebny, nie pisz go. Obejmuje to nawet brak wypełnienia elsew twoim przypadku, gdy kod wraca z ifbloku:

if(condition) 
{
    doStuff();
    return whatever;
}

doSomethingElse(); // no else needed
return somethingelse;

Jeśli chodzi o drugi punkt, dobrze jest unikać nazw boolowskich zawierających negatywne:

bool doorNotOpen = false; // a double negative is hard to reason
bool doorClosed = false; // this is much clearer

Jednak, jak zauważyłeś, rozszerzenie tego zakresu na brak testowania negatywu prowadzi do bardziej bezużytecznego pisania. Poniższe jest o wiele wyraźniejsze niż posiadanie pustego ifbloku:

if(!condition)
{
    doStuff();
    return whatever;
}
David Arno
źródło
120
Więc ... można powiedzieć, że podwójne negatywne to nie-nie? ;)
Patsuan
6
@Patsuan, bardzo sprytny. Lubię to. :)
David Arno
4
Wydaje mi się, że niewiele osób zgodzi się co do tego, jak należy postępować ze zwrotami, a niektórzy mogą nawet powiedzieć, że nie powinno być tak samo w każdym scenariuszu. Sam nie mam silnych preferencji, ale zdecydowanie widzę argument za wieloma innymi sposobami.
Dukeling,
6
Najwyraźniej if (!doorNotOpen)jest okropny. Dlatego nazwy powinny być jak najbardziej pozytywne. if (! doorClosed)jest całkowicie w porządku.
David Schwartz
1
Jeśli chodzi o przykład drzwi, nazwy dwóch przykładów niekoniecznie są równoważne. W świecie fizycznym doorNotOpennie jest to samo, ponieważ doorClosedistnieje stan przejściowy między otwarciem a zamknięciem. Cały czas widzę to w moich aplikacjach przemysłowych, w których stany logiczne są określane przez czujniki fizyczne. Jeśli masz pojedynczy czujnik na otwartej stronie drzwi, możesz tylko powiedzieć, że drzwi są otwarte lub nie są otwarte. Podobnie, jeśli czujnik znajduje się po stronie zamkniętej, możesz powiedzieć tylko zamknięty lub nie zamknięty. Również sam czujnik określa sposób potwierdzania stanu logicznego.
Peter M
45

1. Argument za pustymi elsestwierdzeniami.

Często używam (i argumentuję) czegoś podobnego do tego pierwszego konstruktu, pustego innego. Sygnalizuje to czytelnikom kodu (zarówno ludzkim, jak i automatycznym narzędziom analitycznym), że programista zastanowił się nad sytuacją. Brakujące elseoświadczenia, które powinny być obecne, zabijały ludzi, rozbijały pojazdy i kosztowały miliony dolarów. Na przykład MISRA-C nakazuje przynajmniej komentarz stwierdzający, że brakujący finał jest celowy w if (condition_1) {do_this;} else if (condition_2) {do_that;} ... else if (condition_n) {do_something_else;}sekwencji. Inne standardy wysokiej niezawodności idą nawet dalej: z kilkoma wyjątkami, brak innych instrukcji jest zabronione.

Jednym wyjątkiem jest prosty komentarz, coś podobnego do /* Else not required */. Sygnalizuje to ten sam cel, co trzy linie puste. Kolejnym wyjątkiem, w którym to puste miejsce nie jest potrzebne, jest oczywiste zarówno dla czytelników kodu, jak i dla automatycznych narzędzi analitycznych, które to puste zbędne. Na przykład if (condition) { do_stuff; return; }Podobnie puste pole nie jest potrzebne w przypadku throw somethinglub goto some_label1 zamiast return.

2. Argument za preferowanie if (condition)ponad if (!condition).

To jest czynnik ludzki. Złożona logika boolowska wprawia w ruch wiele osób. Nawet doświadczony programista będzie musiał pomyśleć if (!(complex || (boolean && condition))) do_that; else do_this;. Przynajmniej przepisz to jako if (complex || (boolean && condition)) do_this; else do_that;.

3. Nie oznacza to, że należy preferować puste thenstwierdzenia.

Druga sekcja mówi „wolisz” niż „powinieneś”. Jest to raczej wytyczna niż reguła. Powodem, dla którego wytyczne te preferują pozytywne ifwarunki, jest to, że kod powinien być jasny i oczywisty. Klauzula pusta wtedy (np. if (condition) ; else do_something;) Narusza to. Jest to zaciemnione programowanie, które powoduje, że nawet najbardziej doświadczeni programiści wykonują kopie zapasowe i ponownie czytają ifstan w negowanej formie. Więc napisz to w negacji w pierwszej kolejności i pomiń instrukcję else (lub miej puste pole else lub komentarz na ten temat, jeśli jest do tego upoważniony).



1 pisałem, że wtedy klauzule, które kończą się return, throwczy gotonie wymagają pusty indziej. Oczywiste jest, że klauzula else nie jest potrzebna. Ale co z goto? Nawiasem mówiąc, zasady programowania krytyczne dla bezpieczeństwa czasami zabraniają wczesnego powrotu i prawie zawsze nie dopuszczają wyjątków. Pozwalają jednak gotona to w ograniczonej formie (np goto cleanup1;.). To ograniczone użycie gotojest preferowaną praktyką w niektórych miejscach. Na przykład jądro Linuksa jest pełne takich gotoinstrukcji.

David Hammen
źródło
6
!również łatwo przeoczyć. To jest zbyt zwięzłe.
Thorbjørn Ravn Andersen
4
Nie, puste pole nie wyjaśnia, że ​​programista zastanowił się nad tym. Prowadzi to do większego zamieszania „Czy zaplanowano jakieś oświadczenia i zapomnieli lub dlaczego istnieje pusta klauzula?” Komentarz jest znacznie jaśniejszy.
Simon
9
@ Simon: Typowa forma to else { /* Intentionally empty */ }coś w tym stylu. To, co puste, spełnia wymagania analizatora statycznego, który bezmyślnie szuka naruszeń zasad. Komentarz informuje czytelników, że puste inne jest zamierzone. Z drugiej strony, doświadczeni programiści zwykle pomijają komentarz jako niepotrzebny - ale nie jest pusty. Programowanie o wysokiej niezawodności jest domeną samą w sobie. Konstrukty wyglądają raczej dziwnie dla osób postronnych. Te konstrukty są używane z powodu lekcji ze szkoły mocnych uderzeń, uderzeń, które są bardzo trudne, gdy życie i śmierć lub $$$$$$$$$ są pod ręką.
David Hammen,
2
Nie rozumiem, jak puste są wtedy klauzule, co jest złą rzeczą, gdy puste klauzule nie są (zakładając, że wybraliśmy ten styl). Widzęif (target == source) then /* we need to do nothing */ else updateTarget(source)
Bergi,
2
@ ThorbjørnRavnAndersen nope, notpodobnie jak inne operatory bitowe i logiczne , jest słowem kluczowym w C ++. Zobacz alternatywne reprezentacje operatorów .
Ruslan
31

Używam pustej gałęzi else (a czasem pustej gałęzi if) w bardzo rzadkich przypadkach: gdy jest oczywiste, że zarówno część if, jak i część powinny być obsługiwane w jakiś sposób, ale z jakiegoś nietrywialnego powodu sprawa może być obsługiwana przez nic nie robić. Dlatego każdy, kto czyta kod z koniecznym działaniem else, natychmiast podejrze, że czegoś brakuje i zmarnuje swój czas.

if (condition) {
    // No action needed because ...
} else {
    do_else_action()
}

if (condition) {
    do_if_action()
} else {
    // No action needed because ...
}

Ale nie:

if (condition) {
    do_if_action()
} else {
    // I was told that an if always should have an else ...
}
gnasher729
źródło
5
To. Uważam, że instrukcja if test succeeds -> no action needed -> else -> action neededjest semantycznie bardzo różna od instrukcji if it applies that the opposite of a test outcome is true then an action is needed. Przypomina mi się cytat Martina Fowlera: „każdy może pisać kod, który komputery rozumieją; dobrzy programiści piszą kod, który ludzie rozumieją”.
Tasos Papastylianou
2
@TasosPapastylianou To oszustwo. Przeciwieństwem „jeśli test się powiedzie -> nie jest wymagana akcja” jest „jeśli test się nie powiedzie -> akcja jest konieczna”. Wypełniłeś go około 10 słowami.
Jerry B
@JerryB zależy od „testu”. Czasami negacja jest (językowa) prosta, ale czasem nie jest i prowadziłaby do zamieszania. W takich przypadkach łatwiej jest mówić o „zaprzeczeniu / odwrotności prawdziwego wyniku” w porównaniu z „nieprawdziwym wynikiem”; Chodzi jednak o to, że jeszcze łatwiej byłoby wprowadzić „pusty” krok lub odwrócić znaczenie nazw zmiennych. Jest to typowe w sytuacjach „de Morgan”: np. if ((missing || corrupt) && !backup) -> skip -> else do stuffJest o wiele jaśniejsze (+ inne dorozumiane zamiary) niżif ((!missing && !corrupt) || backup) -> do stuff
Tasos Papastylianou
1
@TasosPapastylianou Możesz napisać if(!((missing || corrupt) && !backup)) -> do stufflub lepiej if((aviable && valid) || backup) -> do stuff. (Ale w prawdziwym przykładzie przed użyciem należy sprawdzić, czy kopia zapasowa jest ważna)
12431234123412341234123
2
@TasosPapastylianou gdzie są podwójne negatywy w tym, co powiedziałem? Jasne, jeśli używałbyś niepoprawnej nazwy zmiennej, miałbyś jasny punkt. Chociaż, jeśli chodzi o takie mieszane operatory boolowskie, to może być lepiej mieć zmienne tymczasowe do użycia w if: file_ok = !missing && !corrupt; if(file_ok || backup) do_stuff();co, moim zdaniem, sprawia, że ​​jest jeszcze wyraźniej.
Baldrickk
23

Wszystkie pozostałe są równe, wolę zwięzłość.

Czego nie piszesz, nikt nie musi czytać i rozumieć.

Chociaż wyraźne może być przydatne, dzieje się tak tylko wtedy, gdy bez nadmiernej gadatliwości stanie się oczywiste, że to, co napisałeś, jest naprawdę tym, co chciałeś napisać.


Unikaj więc pustych gałęzi, są one nie tylko bezużyteczne, ale także rzadkie i prowadzą do zamieszania.

Unikaj także pisania gałęzi else, jeśli wyjdziesz bezpośrednio z gałęzi if.

Przydatnym zastosowaniem jawności byłoby umieszczenie komentarza za każdym razem, gdy przejdziemy przez skrzynkę przełączników // FALLTHRU, oraz użycie komentarza lub pustego bloku, w którym potrzebujesz pustej instrukcji for(a;b;c) /**/;.

Deduplikator
źródło
7
// FALLTHRUUgh, nie w SCREAMING CAPS lub w skrótach txtspk. W przeciwnym razie sam się zgadzam i stosuję tę praktykę.
Cody Gray,
7
@CodyGray - To jest jedno miejsce, w którym SHOUTING to dobry pomysł. Większość instrukcji switch kończy każdą literę na break. Brak, breakktóry doprowadził do spektakularnych awarii oprogramowania. Dobrym pomysłem jest komentarz, który krzyczy czytelnikom kodu, że celowe przejście jest celowe. To, że narzędzia analizy statycznej mogą szukać tego konkretnego wzorca i nie narzekać na kryzys, czyni go jeszcze lepszym.
David Hammen,
1
@Wossname: Właśnie sprawdziłem moją vimi nie rozpoznaje żadnej odmiany FALLTHRU. I myślę, że nie bez powodu: TODOa FIXMEkomentarze są komentarze, które muszą być grepable, ponieważ chcemy, aby móc zapytać git grep, które znane wady masz w kodzie, a gdzie są one zlokalizowane. Kryzys nie jest wadą i nie ma powodu, by się za niego żałować.
cmaster
4
@DavidHammen Nie, krzyczenie nie jest dobrym pomysłem: jeśli masz // przełom w miejscu oczekiwanej przerwy ;, to powinno wystarczyć, aby każdy programista natychmiast zrozumiał. Ponadto // przegląd nie mówi o defekcie, po prostu sygnalizuje, że okoliczności zostały wzięte pod uwagę i że przerwa; które wydaje się być zaginione, naprawdę nie ma go tam być. Jest to cel czysto informacyjny i nie ma o czym krzyczeć.
cmaster
1
@cmaster - Twoje zdanie nie jest dobrym pomysłem . Opinie innych ludzi są różne. I tylko dlatego, że twoja konfiguracja vima nie rozpoznaje FALLTHRU, nie oznacza, że ​​inni ludzie nie skonfigurowali vima, aby to zrobić.
David Hammen,
6

Według mojej wiedzy nie ma twardych i szybkich zasad dotyczących pozytywnych lub negatywnych warunków dla instrukcji IF. Osobiście wolę kodowanie dla przypadku pozytywnego niż negatywnego, w stosownych przypadkach. Z pewnością tego nie zrobię, jeśli doprowadzi mnie to do utworzenia pustego bloku JEŻELI, po którym następuje ELSE pełen logiki. Gdyby taka sytuacja się pojawiła, zajęłoby to 3 sekundy, aby powrócić do testowania pod kątem pozytywnego przypadku.

To, co naprawdę nie lubię w twoich przykładach, to całkowicie niepotrzebna przestrzeń pionowa zajmowana przez pusty ELSE. Po prostu nie ma po temu żadnego powodu. Nie dodaje niczego do logiki, nie pomaga dokumentować tego, co robi kod, i wcale nie zwiększa czytelności. W rzeczywistości argumentowałbym, że dodana przestrzeń pionowa mogłaby prawdopodobnie zmniejszyć czytelność.

Langecrew
źródło
2
Ta niepotrzebna przestrzeń pionowa jest konsekwencją mandatów dotyczących zawsze używania nawiasów klamrowych, niedozwolania brakowania innych elementów oraz używania stylu Allmana do wcięcia.
David Hammen,
Cóż, pomyślałbym, że zawsze używanie nawiasów klamrowych jest dobrą rzeczą, ponieważ wyraźnie określa intencje programistów - nie ma miejsca na zgadywanie podczas czytania ich kodu. Może to zabrzmieć głupio, ale zaufaj mi, zwłaszcza gdy mentorujesz młodszych programistów, zdarzają się błędy związane z brakującymi nawiasami klamrowymi. Osobiście nigdy nie byłem fanem wcięć w stylu Allmana, właśnie z tego powodu, o którym wspomniałeś: niepotrzebnej przestrzeni pionowej. Ale to tylko moja opinia i osobisty styl. Właśnie tego nauczyłem się na początku, więc zawsze było mi wygodnie czytać.
Langecrew,
Styl osobisty: niezależnie od tego, jakiego stylu nawiasów klamrowych używam (Allman, 1 TB, ...), zawsze używam nawiasów klamrowych.
David Hammen,
Skomplikowane warunki warunkowe lub te, które Twoim zdaniem mogą być trudne do zrozumienia, można prawie zawsze rozwiązać poprzez ponowne uwzględnienie ich własnych metod / funkcji. Coś takiego if(hasWriteAccess(user,file))może być skomplikowane, ale na pierwszy rzut oka wiesz dokładnie, jaki powinien być wynik. Nawet jeśli jest to tylko kilka warunków, np. if(user.hasPermission() && !file.isLocked())Wywołanie metody o odpowiedniej nazwie ma sens, więc negatywy stają się mniejszym problemem.
Chris Schneider,
Zgoda. Z drugiej strony jestem wielkim fanem refaktoryzacji, gdy tylko jest to możliwe :)
Langecrew,
5

Jawny elseblok

Nie zgadzam się z tym jako ogólne stwierdzenie obejmujące wszystkie ifstwierdzenia, ale zdarza się, że dodanie elsebloku z przyzwyczajenia jest dobrą rzeczą.

ifOświadczenie, moim zdaniem, w rzeczywistości obejmuje dwie odrębne funkcje.

Jeśli mamy coś zrobić, zrób to tutaj.

Takie rzeczy oczywiście nie wymagają elseczęści.

    if (customer.hasCataracts()) {
        appointmentSuggestions.add(new CataractAppointment(customer));
    }
    if (customer.isDiabetic()) {
        customer.assignNurse(DiabeticNurses.pickBestFor(customer));
    }

aw niektórych przypadkach naleganie na dodanie elsemoże wprowadzić w błąd.

    if (k > n) {
        return BigInteger.ZERO;
    }
    if (k <= 0 || k == n) {
        return BigInteger.ONE;
    }

to nie to samo co

    if (k > n) {
        return BigInteger.ZERO;
    } else {
        if (k <= 0 || k == n) {
            return BigInteger.ONE;
        }
    }

nawet jeśli jest funkcjonalnie taki sam. Pisanie pierwszego ifz pustym elsemoże doprowadzić do drugiego wyniku, który jest niepotrzebnie brzydki.

Jeśli sprawdzamy konkretny stan, często dobrym pomysłem jest dodanie pustego, elseaby przypomnieć o sytuacji

        // Count wins/losses.
        if (doors[firstChoice] == Prize.Car) {
            // We would have won without switching!
            winWhenNotSwitched += 1;
        } else {
            // We win if we switched to the car!
            if (doors[secondChoice] == Prize.Car) {
                // We picked right!
                winWhenSwitched += 1;
            } else {
                // Bad choice.
                lost += 1;
            }
        }

Pamiętaj, że te zasady obowiązują tylko podczas pisania nowego kodu . IMHO Puste elseklauzule powinny zostać usunięte przed odprawą.


Testuj true, a niefalse

Ponownie jest to dobra rada na poziomie ogólnym, ale w wielu przypadkach powoduje to, że kod jest niepotrzebnie złożony i mniej czytelny.

Mimo że kod podobny

    if(!customer.canBuyAlcohol()) {
        // ...
    }

denerwuje czytelnika, ale robi to

    if(customer.canBuyAlcohol()) {
        // Do nothing.
    } else {
        // ...
    }

jest co najmniej tak źle, jeśli nie gorzej.

I kodowane w BCPL wiele lat temu iw tym języku istnieje IFklauzula iUNLESS klauzula więc można kodować wiele bardziej czytelnie jako:

    unless(customer.canBuyAlcohol()) {
        // ...
    }

co jest znacznie lepsze, ale wciąż nie idealne.


Mój osobisty proces

Zasadniczo, gdy piszę nowy kod, często dodaję pusty elseblok do ifinstrukcji, aby przypomnieć mi, że nie opisałem jeszcze tej ewentualności. Pomaga mi to uniknąć DFSpułapki i zapewnia, że ​​kiedy przeglądam kod, zauważam, że jest więcej do zrobienia. Zazwyczaj jednak dodaję TODOkomentarz, aby śledzić.

  if (returnVal == JFileChooser.APPROVE_OPTION) {
    handleFileChosen();
  } else {
    // TODO: Handle case where they pressed Cancel.
  }

Uważam, że generalnie elserzadko używam w kodzie, ponieważ często może to wskazywać na zapach kodu.

OldCurmudgeon
źródło
Twoja obsługa „pustych” bloków czyta się jak standardowy kod usuwający kod podczas implementacji thngs ...
Deduplicator
unlessBlok to dobra propozycja, a ledwie ograniczone do BCPL.
tchrist
@TobySpeight Ups. Jakoś umknęło mojej uwadze, że to, co napisałem, ...było w rzeczywistości return. (W rzeczywistości, z k=-1, n=-2pierwszy powrót jest brany, ale jest to w dużej mierze dyskusyjne.)
David Richerby
Modern Perl ma ifi unless. Co więcej, umożliwia to również po oświadczeniu, dzięki czemu możesz powiedzieć print "Hello world!" if $born;lub print "Hello world!" unless $dead;oba zrobią dokładnie to, co według ciebie robią.
CVn
1

Po pierwsze, użyłem języka, który wymusił użycie instrukcji IF w ten sposób (w Opal, język za skrobaczką do ekranu głównego do umieszczania interfejsu użytkownika GUI w systemach mainframe) i tylko z jedną linią dla IF i ELSE. To nie było przyjemne doświadczenie!

Spodziewałbym się, że każdy kompilator zoptymalizuje takie dodatkowe klauzule ELSE. Ale w przypadku kodu na żywo nic nie dodaje (w fazie rozwoju może być użytecznym znacznikiem dla dalszego kodu).

Czasem używam czegoś takiego jak te dodatkowe klauzule, kiedy używam przetwarzania typu CASE / WHEN. Zawsze dodam domyślną klauzulę, nawet jeśli jest pusta. Jest to długotrwały nawyk ze strony języków, które popełniają błąd, jeśli taka klauzula nie zostanie użyta, i zmusza do zastanowienia się, czy rzeczy naprawdę powinny po prostu przejść.

Dawno temu praktyka na komputerach mainframe (np. PL / 1 i COBOL) została przyjęta, że ​​kontrole negatywne były mniej skuteczne. To może wyjaśnić drugi punkt, chociaż w dzisiejszych czasach są znacznie ważniejsze oszczędności wydajności, które są ignorowane jako mikrooptymalizacje.

Negatywna logika bywa mniej czytelna, choć nie tak bardzo w przypadku tak prostej instrukcji IF.

Rozpocznij
źródło
2
Rozumiem, więc w pewnym momencie była to powszechna praktyka.
Patsuan
@Patsuan - tak, do pewnego stopnia. Chociaż właśnie wtedy asembler komputerów mainframe był poważną alternatywą dla kodu krytycznego dla wydajności.
Kickstart,
1
„Dawno temu ... przyjęto, że kontrole negatywne były mniej skuteczne.” Dobrze, że chyba kompilator optymalizuje if !A then B; else Csię if A then C; else B. Obliczanie negacji warunku jest dodatkową instrukcją procesora. (Chociaż, jak mówisz, jest mało prawdopodobne, aby było to znacznie mniej wydajne.)
David Richerby
@DavidRicherby A jeśli mówimy ogólnie, niektóre języki mogą utrudnić takie optymalizacje. Weźmy na przykład C ++ z przeciążonymi !i ==operatorami. Nie chodzi o to, że ktoś powinien to robić , pamiętajcie o tym ...
CV z
1

Oparłbym stanowisko większości odpowiedzi, że puste elsebloki są praktycznie zawsze szkodliwym marnotrawstwem atramentu elektronicznego. Nie dodawaj ich, chyba że masz ku temu dobry powód, w którym to przypadku pusty blok wcale nie powinien być pusty, powinien zawierać komentarz wyjaśniający, dlaczego tak jest.

Problem unikania negatywów zasługuje jednak na większą uwagę: zazwyczaj, gdy trzeba użyć wartości logicznej, potrzebujesz bloków kodu do działania, gdy jest on ustawiony, oraz innych bloków do działania, gdy nie jest ustawiony. W związku z tym, jeśli wymusisz regułę nieujemną, wymuszasz albo posiadanie if() {} else {...}instrukcji (z pustym ifblokiem!), Albo tworzysz drugą wartość logiczną dla każdej wartości logicznej, która zawiera jej zanegowaną wartość. Obie opcje są złe, ponieważ dezorientują czytelników.

Pomocna zasada jest taka: Nigdy nie używaj zanegowanej formy w nazwie logicznej i wyrażaj negację jako pojedynczą !. Takie zdanie if(!doorLocked)jest całkowicie jasne, takie jak if(!doorUnlocked)mózg węzłów. Późniejszy rodzaj wyrażenia to rzecz, której należy unikać za wszelką cenę, a nie obecność jednego !w danym if()stanie.

cmaster
źródło
0

Powiedziałbym, że to zdecydowanie zła praktyka. Dodanie innych instrukcji spowoduje dodanie do kodu całego szeregu bezcelowych wierszy, które nic nie robią, a jeszcze bardziej utrudnią ich odczytanie.

ayrton clark
źródło
1
Słyszałem, że nigdy nie pracowałeś dla pracodawcy, który ocenia twoje wyniki na podstawie SLOC wyprodukowanych w danym okresie ...
CV
0

Jest jeszcze jeden wzorzec, który nie został jeszcze wspomniany na temat obsługi drugiego przypadku, który ma pusty blok if. Jednym ze sposobów radzenia sobie z tym jest powrót do bloku if. Lubię to:

void foo()
{
    if (condition) return;

    doStuff();
    ...
}

Jest to powszechny wzorzec sprawdzania warunków błędów. Przypadek twierdzący jest nadal używany, a jego wnętrze nie jest już puste, co pozwala przyszłym programistom zastanawiać się, czy rezygnacja nie była zamierzona. Może także poprawić czytelność, ponieważ może być konieczne utworzenie nowej funkcji (dzielenie dużych funkcji na mniejsze indywidualne funkcje).

Shaz
źródło
0

Rozważając argument „zawsze mam inną klauzulę”, jest jeden punkt, którego nie widziałem w żadnej innej odpowiedzi: może mieć sens w funkcjonalnym stylu programowania. Raczej.

W funkcjonalnym stylu programowania operujesz wyrażeniami, a nie wyrażeniami. Zatem każdy blok kodu ma wartość zwracaną - w tym if-then-elsewyrażenie. Wykluczałoby to jednak pusty elseblok. Dam ci przykład:

var even = if (n % 2 == 0) {
  return "even";
} else {
  return "odd";
}

Teraz w językach ze składnią w stylu C lub inspirowaną stylem C (takich jak Java, C # i JavaScript, żeby wymienić tylko kilka), wygląda to dziwnie. Wygląda jednak znacznie lepiej, gdy jest napisany jako taki:

var even = (n % 2 == 0) ? "even" : "odd";

Pozostawienie elsetutaj pustej gałęzi spowodowałoby, że wartość byłaby niezdefiniowana - w większości przypadków nie jest to prawidłowy przypadek podczas programowania funkcji. To samo z całkowitym pominięciem. Jednak, gdy programujesz iteracyjnie, mam bardzo mało powodów, aby zawsze mieć elseblok.

blalasaadri
źródło
0

... po instrukcji if powinna następować instrukcja else, niezależnie od tego, czy jest coś do wprowadzenia, czy nie.

Nie zgadzam się, że if (a) { } else { b(); }powinienem być przepisany jako if (!a) { b(); }i if (a) { b(); } else { }powinien zostać przepisany jako if (a) { b(); }.

Warto jednak zaznaczyć, że rzadko mam pustą gałąź. Jest tak, ponieważ normalnie loguję, że wszedłem do innej pustej gałęzi. W ten sposób mogę opracować wyłącznie komunikaty dziennika. Rzadko używam debugera. W środowiskach produkcyjnych nie ma debugera; miło jest móc rozwiązywać problemy produkcyjne za pomocą tych samych narzędzi, których używasz do programowania.

Po drugie, lepiej jest sprawdzić prawdziwe wartości niż fałsz. Oznacza to, że lepiej jest przetestować zmienną „doorClosed” zamiast zmiennej „! DoorOpened”

Mam mieszane uczucia na ten temat. Wadą posiadania doorClosedi doorOpenedpotencjalnie podwojenia liczby słów / terminów, o których musisz wiedzieć. Kolejną wadą jest z czasem znaczenie doorClosedi doorOpenedmoże się zmieniać (inni programiści przychodzą za tobą) i możesz skończyć z dwiema właściwościami, które nie są już dokładnymi zaprzeczeniami. Zamiast unikać negacji, cenię sobie dostosowanie języka kodu (nazw klas, nazw zmiennych itp.) Do słownictwa użytkowników biznesowych i do wymagań, które otrzymałem. Nie chciałbym wymyślać zupełnie nowego terminu, aby uniknąć!jeśli termin ten ma znaczenie tylko dla programisty, którego użytkownicy biznesowi nie zrozumieją. Chcę, aby programiści mówili tym samym językiem, co użytkownicy i osoby piszące wymagania. Teraz, jeśli nowe warunki upraszczają model, to ważne, ale należy się tym zająć przed sfinalizowaniem wymagań, a nie później. Projektanci powinni poradzić sobie z tym problemem, zanim zaczną kodować.

Jestem skłonny wątpić w mój instynkt.

Dobrze jest nadal zadawać sobie pytania.

Czy to dobra / zła / „nieważna” praktyka?

Do pewnego stopnia wiele z tego nie ma znaczenia. Sprawdź kod i dostosuj się do swojego zespołu. Zazwyczaj jest to właściwe. Jeśli wszyscy w twoim zespole chcą kodować w określony sposób, prawdopodobnie najlepiej to zrobić w ten sposób (zmiana 1 osoby - ciebie - zajmuje mniej punktów historii niż zmiana grupy ludzi).

Czy to powszechna praktyka?

Nie pamiętam, żeby widziałem pustą gałąź. Jednak widzę ludzi dodających właściwości, aby uniknąć negacji.

Słowa jak Jared
źródło
0

Zajmę się tylko drugą częścią pytania, a to wynika z mojego punktu widzenia pracy w systemach przemysłowych i manipulowania urządzeniami w prawdziwym świecie.

Jest to jednak w pewnym sensie rozszerzony komentarz, a nie odpowiedź.

Kiedy jest powiedziane

Po drugie, lepiej jest sprawdzić prawdziwe wartości niż fałsz. Oznacza to, że lepiej jest przetestować zmienną „doorClosed” zamiast zmiennej „! DoorOpened”

Jego argumentem jest to, że wyjaśnia on, co robi kod.

Z mojego punktu widzenia przyjmuje się tutaj założenie, że stan drzwi jest stanem binarnym, podczas gdy w rzeczywistości jest to przynajmniej stan potrójny:

  1. Drzwi są otwarte.
  2. Drzwi nie są całkowicie otwarte ani całkowicie zamknięte.
  3. Drzwi są zamknięte.

Zatem w zależności od tego, gdzie znajduje się pojedynczy, binarny czujnik cyfrowy, można przypuszczać tylko jedną z dwóch koncepcji:

  1. Jeśli czujnik znajduje się po otwartej stronie drzwi: drzwi są otwarte lub nie są otwarte
  2. Jeśli czujnik znajduje się po zamkniętej stronie drzwi: drzwi są albo zamknięte, albo nie zamknięte.

I nie można wymieniać tych pojęć za pomocą binarnej negacji. Tak więc doorClosedi !doorOpenedprawdopodobnie nie są synonimami, a każda próba udawania, że ​​są synonimami, jest błędnym myśleniem, które zakłada większą wiedzę o stanie systemu niż w rzeczywistości istnieje.

Wracając do twojego pytania, poparłbym verbage, które pasuje do źródła informacji reprezentowanej przez zmienną. Tak więc, jeśli informacje pochodzą z zamkniętej strony drzwi, należy przejść z doorCloseditd. Może to oznaczać użycie jednego doorClosedlub !doorClosedw razie potrzeby w różnych oświadczeniach zgodnie z wymaganiami, co może spowodować potencjalne pomylenie z użyciem negacji. Ale to, czego nie robi, to pośrednio propaguje założenia dotyczące stanu systemu.


Do celów dyskusji na temat mojej pracy ilość dostępnych informacji o stanie systemu zależy od funkcjonalności wymaganej przez sam system.

Czasami muszę tylko wiedzieć, czy drzwi są zamknięte, czy nie zamknięte. W takim przypadku wystarczy pojedynczy czujnik binarny. Ale innym razem muszę wiedzieć, że drzwi są otwarte, zamknięte lub w trakcie przejścia. W takich przypadkach byłyby albo dwa czujniki binarne (na każdym krańcu ruchu drzwi - z kontrolą błędów przy jednoczesnym otwieraniu i zamykaniu drzwi) lub istniałby czujnik analogowy mierzący, jak „otwarte” były drzwi.

Przykładem pierwszego są drzwi do kuchenki mikrofalowej. Umożliwiają działanie piekarnika w zależności od tego, czy drzwiczki są zamknięte lub nie są zamknięte. Nie obchodzi cię, jak otwarte są drzwi.

Przykładem drugiego może być prosty siłownik napędzany silnikiem. Uniemożliwia się napędzanie silnika do przodu, gdy siłownik jest całkowicie wysunięty. I hamujesz napędzanie silnika do tyłu, gdy siłownik jest całkowicie włączony.

Zasadniczo liczba i rodzaj czujników sprowadza się do przeprowadzenia analizy kosztów czujników względem analizy wymagań dotyczących tego, co jest potrzebne do osiągnięcia wymaganej funkcjonalności.

Peter M.
źródło
„Z mojego punktu widzenia przyjmuje się tutaj założenie, że stan drzwi jest stanem binarnym, podczas gdy w rzeczywistości jest to co najmniej stan trójskładnikowy:” i „Umożliwiacie działanie piekarnika w oparciu o to, że drzwi są zamknięte lub nie są zamknięte Nie obchodzi cię, jak otwarte są drzwi. ” zaprzeczają sobie. Nie sądzę też, aby pytanie dotyczyło w ogóle drzwi i czujników.
Sanchises
@ Sanchises Stwierdzenia nie są sprzeczne, ponieważ zostały wyjęte z kontekstu. Pierwsze stwierdzenia są w kontekście, że dwa przeciwne pomiary binarne stanu trójskładnikowego nie mogą być zamienione przez negację binarną. Drugie stwierdzenie jest w kontekście, że tylko częściowa znajomość stanu trójskładnikowego może być wystarczająca do poprawnego działania aplikacji. Jeśli chodzi o drzwi, pytanie PO dotyczyło drzwi otwieranych i zamykanych. Wskazuję tylko oczywiste założenie, które może wpłynąć na sposób przetwarzania danych.
Peter M
-1

Konkretne reguły stylu są zawsze złe. Wytyczne są dobre, ale w końcu często zdarza się, że możesz zrobić coś jaśniejszego, robiąc coś, co nie jest zgodne z wytycznymi.

To powiedziawszy, jest wiele nienawiści do pustych jeszcze „marnuje linie” „dodatkowego pisania”, które są po prostu złe. Możesz argumentować za przeniesieniem nawiasów na tę samą linię, jeśli naprawdę potrzebujesz przestrzeni pionowej, ale jeśli to jest problem, nie powinieneś umieszczać {w osobnej linii.

Jak wspomniano w innym miejscu, blok else jest bardzo przydatny, aby pokazać, że wyraźnie nie chcesz, aby nic się nie wydarzyło w innym przypadku. Po zrobieniu dużo programowania funkcjonalnego (gdzie jest to wymagane), nauczyłem się, że zawsze powinieneś brać pod uwagę inne przy pisaniu if, chociaż, jak wspomina @OldCurmudgeon, istnieją naprawdę dwa różne przypadki użycia. Trzeba mieć coś innego, nie należy. Niestety, nie jest to coś, co zawsze można powiedzieć na pierwszy rzut oka, nie mówiąc już o wkładce, stąd dogmatyczne „zawsze stawiaj inny blok”.

Jeśli chodzi o „brak negatywów”, ponownie, bezwzględne zasady są złe. Posiadanie pustego, jeśli może być dziwne, szczególnie jeśli jest to rodzaj, jeśli nie potrzebuje innego, więc pisz to wszystko, a nie! lub an == fałsz jest zły. To powiedziawszy, istnieje wiele przypadków, w których negatywne ma sens. Typowym przykładem może być buforowanie wartości:

static var cached = null

func getCached() {
    if !cached {
        cached = (some calculation, etc)
    }

    return cached
}

jeśli rzeczywista (mentalna / angielska) logika obejmuje negatywne, to samo powinno być w przypadku instrukcji if.

zacaj
źródło