Utrzymywalność logiki boolowskiej - czy zagnieżdżanie jest konieczne, jeśli instrukcje są potrzebne?

11

Który z nich jest lepszy dla utrzymania?

if (byteArrayVariable != null)
    if (byteArrayVariable .Length != 0)
         //Do something with byteArrayVariable 

LUB

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

Wolę czytać i pisać drugie, ale pamiętam, że czytanie w kodzie jest kompletne, że robienie takich rzeczy jest niekorzystne z punktu widzenia konserwacji.

Wynika to z faktu, że polegasz na języku, aby nie oceniać drugiej części, ifjeśli pierwsza część jest fałszywa i nie wszystkie języki to robią. (Druga część zgłosi wyjątek, jeśli zostanie oceniony z wartością null byteArrayVariable).

Nie wiem, czy naprawdę jest to coś do zmartwienia, czy nie, i chciałbym uzyskać ogólną opinię na temat tego pytania.

Dzięki.

Vaccano
źródło
1
Pierwsza forma grozi zwisaniem innych
JBRWilkinson
tylko ciekawy, z jakim językiem pracujesz?
STW,
@STW - używamy C # i mamy trochę starszego kodu w Delphi.
Vaccano
2
dobrze wiedzieć. Nie jestem zaznajomiony z Delphi, ale w C # można mieć pewność, w &&i ||operatorów przeprowadzających ocenę zwarciem. Przechodzenie z jednego języka do drugiego zwykle ma pewne problemy i dobrze jest być stanowczym w kwestii dokonywania recenzji kodu, aby pomóc złapać te małe problemy.
STW,
Podczas gdy wraz z wieloma odpowiedziami tutaj uważam, że druga jest bardziej czytelna, pierwsza jest o wiele łatwiejsza do debugowania. W tym przypadku nadal używałbym drugiego, ponieważ sprawdzasz, jak łatwo debugować rzeczy. Ale ogólnie bądź ostrożny.
Mag

Odpowiedzi:

30

Myślę, że druga forma jest w porządku, a także jaśniej reprezentuje to, co próbujesz zrobić. Mówisz tak...

Pamiętam, że przeczytałem w kodzie kompletne, że robienie takich rzeczy źle wpływa na łatwość konserwacji. Wynika to z faktu, że polegasz na języku, aby nie oceniać drugiej części, jeśli pierwsza część jest fałszywa i nie wszystkie języki to robią.

Nie ma znaczenia, czy robią to wszystkie języki. Piszesz w jednym konkretnym języku, więc ma to znaczenie tylko wtedy , gdy ten język to robi. W przeciwnym razie zasadniczo mówisz, że nie powinieneś używać funkcji konkretnego języka, ponieważ inne języki mogą nie obsługiwać tych funkcji, a to po prostu głupie.

mipadi
źródło
12
Zgoda. Dlaczego, do cholery, miałbym przejmować się tym, że mój kod będzie działał tak samo po skopiowaniu na inny język?
Tim Goodman,
16

Dziwi mnie, że nikt o tym nie wspominał (mam nadzieję, że nie mylę się z pytaniem) - ale oba są do bani!

Lepszą alternatywą byłoby użycie wcześniejszych wyjść (umieszczenie instrukcji return na początku kodu, jeśli żaden inny kod nie powinien zostać wykonany). To powoduje założenie, że twój kod jest stosunkowo dobrze refaktoryzowany i że każda metoda ma zwięzły cel.

W tym momencie zrobiłbyś coś takiego:

if ((byteArrayVariable == null) || (byteArrayVariable.Length != 0)) {
  return; // nothing to be done, leaving
}

// Conditions met, do something

Może to wyglądać podobnie do sprawdzania poprawności - i dokładnie tak jest. Potwierdza, że ​​metoda spełniła swoje warunki - obie oferowane opcje ukryły rzeczywistą logikę w ramach kontroli warunków wstępnych.

Jeśli twój kod zawiera dużo spaghetti (długie metody, wiele zagnieżdżonych ifs z logiką rozproszoną między nimi itp.), Powinieneś skupić się na większym problemie z dostosowaniem kodu przed mniejszymi problemami stylistycznymi. W zależności od środowiska i nasilenia bałaganu istnieje kilka świetnych narzędzi, które mogą pomóc (np. Visual Studio ma funkcję refaktoryzacji „Metoda wyodrębniania”).


Jak wspomina Jim, wciąż jest miejsce na debatę na temat tego, jak zorganizować wczesne wyjście. Alternatywą dla powyższego byłoby:

if (byteArrayVariable == null) 
  return; // nothing to be done, leaving

if (byteArrayVariable.Length != 0)
  return;

// Conditions met, do something
STW
źródło
1
Zgadzam się z powrotem zamiast owijania dobrej logiki, ale ci sami ludzie będą używać tego samego argumentu na temat używania || Jeśli chodzi o &&.
MIA
14

Nie trać czasu na próby pułapkowania dla każdego innego niż ten warunek. Jeśli możesz zdyskwalifikować dane lub warunki, zrób to jak najszybciej.

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
// do something

Pozwala to znacznie łatwiej dostosować kod do nowych warunków

if (byteArrayVariable == null) Exit;
if (byteArrayVariable.Length == 0) Exit;
if (NewCondition == false) Exit;
if (NewerCondition == true) Exit;
// do something
Michael Riley - AKA Gunny
źródło
2
+1 Tak, tak, tak. Logika tak prosta, jak to nie powinno skutkować trudnym kodem. Twoje wnętrzności (pytającego) powinny ci to powiedzieć.
Job
11

Twój przykład jest doskonałym przykładem tego, dlaczego zagnieżdżanie ifsjest złym podejściem dla większości języków, które poprawnie obsługują porównanie boolowskie. Zagnieżdżenie ifspowoduje, że twoja intencja nie jest wcale jasna. Czy wymagane jest, aby drugi ifnatychmiast następował po pierwszym? A może to dlatego, że nie przeprowadziłeś jeszcze innej operacji?

if ((byteArrayVariable != null) && (byteArrayVariable.Length != 0))
  //Do something with byteArrayVariable 

W takim przypadku muszą one być prawie razem. Działają jako strażnicy, aby upewnić się, że nie wykonasz operacji, która spowodowałaby wyjątek. Używając zagnieżdżonego ifs, pozostawiasz do interpretacji osoby podążającej za tobą, czy ta dwójka reprezentuje dwuczęściową straż, czy inną logikę rozgałęzienia. Łącząc je w jeden if, stwierdzam to. O wiele trudniej jest podkraść się do operacji tam, gdzie nie powinna być.

Zawsze będę faworyzować jasne przekazywanie mojej intencji zamiast głupiego twierdzenia, że ​​„nie działa we wszystkich językach”. Zgadnij, co ... thrownie działa również we wszystkich językach, czy to oznacza, że ​​nie powinienem go używać podczas kodowania w C # lub Javie i zamiast tego powinienem polegać na wartościach zwrotnych sygnalizujących wystąpienie problemu?

To powiedziawszy, o wiele bardziej czytelne jest odwrócenie go i po prostu powrót z metody, jeśli jedno z nich nie jest spełnione, jak mówi STW w swojej odpowiedzi.

MIA
źródło
Więcej niż dwa warunki mogą uzasadniać ich własną funkcję, która zwraca wartość zerową.
Job
3

W tym przypadku twoje dwie klauzule są bezpośrednio powiązane, więc preferowana jest druga forma.

Gdyby nie były powiązane (np. Szereg klauzul ochronnych na różnych warunkach), wolałbym pierwszą formę (lub przeformułowałbym metodę o nazwie, która reprezentuje to, co faktycznie reprezentuje warunek złożony).

FinnNk
źródło
1

Jeśli pracujesz w Delphi, pierwszy sposób jest na ogół bezpieczniejszy. (W podanym przykładzie nie ma wiele korzyści z używania zagnieżdżonych elementów if).

Delphi pozwala określić, czy wyrażenia boolowskie oceniają, czy „zwarcie” za pomocą przełączników kompilatora. Wykonanie sposobu nr 1 (zagnieżdżone ifs) pozwala upewnić się, że druga klauzula zostanie wykonana wtedy i tylko wtedy, gdy (i ściśle po niej ) pierwsza klauzula uzyska wartość true.

Frank Shearar
źródło
1
W ciągu 23 lat rozwoju Delphi nigdy nie zmieniłem opcji „Complete boolean eval”. Gdybym wysyłał kod gdzie indziej, umieściłbym {$ BOOLEVAL OFF} lub {$ B-} w urządzeniu.
Gerry,
Ja też nie. Zawsze też używam sprawdzania zasięgu ... ale inni nie. I nie powinno to robić różnicy, chyba że z perspektywy wydajności, ponieważ nie powinieneś używać efektów ubocznych w funkcjach używanych w wyrażeniach boolowskich. Jestem pewien, że ktoś tam robi!
Frank Shearar,
@Gerry: uzgodniony. Jedynym powodem, dla którego możesz chcieć uzyskać pełną ocenę, są działania niepożądane - a działania niepożądane w warunkach warunkowych to zły pomysł!
Loren Pechtel
Ponownie przeczytaj mój komentarz 23 lata powinny mieć 13 lat! Nie mam Tardis
Gerry
1

Drugi styl może się powtórzyć w VBA, ponieważ jeśli pierwszym testem jest wygenerowany obiekt, nadal wykona drugi test i wystąpi błąd. Właśnie przyzwyczaiłem się do VBA, kiedy zajmuję się weryfikacją obiektów, aby zawsze używać pierwszej metody.


źródło
2
To dlatego, że VBA jest okropnym językiem
Falmarri,
@Falmarri, ma w tym jakieś okropne rzeczy (i kilka dobrych rzeczy). Naprawiłbym wiele rzeczy, gdybym miał moje druty.
2
Brak zwartej oceny logicznej jest bardziej spuścizną niż czymkolwiek innym. Nie mam pojęcia, dlaczego od tego nie zaczęli. „Poprawka” VB.NET dla OrElse i AndAlso jest dość denerwująca, ale prawdopodobnie jedyna opcja, aby sobie z tym poradzić bez naruszania wstecznej kompatybilności.
JohnFx,
1

Druga forma jest bardziej czytelna, szczególnie jeśli używasz bloków {} wokół każdej klauzuli, ponieważ pierwsza forma prowadzi do zagnieżdżonych bloków {} i wielu poziomów wcięć, co może być utrudnione do odczytania (musisz policzyć wcięcia dowiedzieć się, gdzie kończy się każdy blok).

o. Nate
źródło
0

Oba są dla mnie czytelne i nie akceptuję argumentu, że powinieneś martwić się o łatwość utrzymania kodu, jeśli chcesz zmienić język.

W niektórych językach druga opcja działa lepiej, więc byłby to wtedy oczywisty wybór.

Rachunek
źródło
0

Jestem z tobą; Robię to w drugą stronę. Dla mnie jest to logicznie wyraźniejsze. Obawa, że ​​niektóre języki wykonają drugą część, nawet jeśli pierwsza oceni na fałsz, wydaje mi się trochę głupia, ponieważ nigdy w życiu nie napisałem programu działającego w „niektórych językach”; mój kod zawsze wydaje się istnieć w określonym języku, który albo może obsłużyć tę konstrukcję, albo nie. (A jeśli nie jestem pewien, czy konkretny język sobie z tym poradzi, muszę się go nauczyć, prawda?)

W mojej opinii ryzyko związane z pierwszym sposobem zrobienia tego polega na tym, że naraża mnie na przypadkowe wprowadzenie kodu poza ten zagnieżdżony ifblok, gdy nie miałem na myśli tego. Co, co prawda, nie stanowi większego problemu, ale wydaje się bardziej rozsądne niż martwienie się tym, czy mój kod jest agnostyczny.

BlairHippo
źródło
0

Wolę drugą opcję, ponieważ jest bardziej czytelna.

Jeśli logika, którą implementujesz, jest bardziej złożona (sprawdzanie, czy łańcuch nie jest pusty i nie jest pusty, to tylko przykład), możesz dokonać przydatnego refaktoryzacji: wyodrębnij funkcję boolowską. Jestem z @mipadi w sprawie „Code Complete” („nie ma znaczenia, że ​​wszystkie języki to robią ...”) Jeśli czytnik twojego kodu nie jest zainteresowany zaglądaniem do wyodrębnionej funkcji, to kolejność w którym są analizowane operandy logiczne, staje się dla nich spornym pytaniem.

azheglov
źródło
0
if ((byteArrayVariable != null)
 && (byteArrayVariable.Length != 0)) {
    //Do something with byteArrayVariable 

ale to w zasadzie druga opcja.

Bojkot SE dla Moniki Cellio
źródło