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

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.

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.

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

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

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.

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).

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.

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.

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).

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.

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.

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.

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

ale to w zasadzie druga opcja.