Czy programiści powinni używać zmiennych boolowskich do „dokumentowania” swojego kodu?

79

Czytam Code Complete McConella , a on omawia użycie zmiennych boolowskich do dokumentowania kodu. Na przykład zamiast:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
       ...
}

On sugeruje:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

Wydaje mi się to logiczne, dobre i bardzo samodokumentujące. Jednak waham się, czy zacząć regularnie używać tej techniki, ponieważ prawie nigdy się z nią nie spotkałem; i być może byłoby to mylące tylko dlatego, że jest rzadkie. Jednak moje doświadczenie nie jest jeszcze zbyt rozległe, więc interesuje mnie opinia programistów na temat tej techniki i byłbym ciekawy, czy ktoś używa tej techniki regularnie lub często ją widzi podczas czytania kodu. Czy warto zastosować konwencję / styl / technikę? Czy inni programiści to zrozumieją i docenią, czy też uznają to za dziwne?

froadie
źródło
@Paul R - ups, dzięki. Zacząłem od samodokumentacji i zdałem sobie sprawę, że nie ma na to żadnego tagu, więc spróbowałem go zmienić na samodokumentowanie, które już istniało. :)
froadie
Czy nie jest lepsze samo komentowanie tego, co dzieje się podczas testu?
JS
3
W razie potrzeby staram się to również zrobić, zwłaszcza jeśli muszę sprawdzić stan (y) w różnych częściach metody. Jest też dużo bardziej opisowy.
geffchang
„Wydaje mi się, że to logiczne, dobre praktyki i bardzo samodokumentujące się”. Opierając się na swoim doświadczeniu, są szanse, że będzie to tak samo łatwe do zrozumienia dla innych programistów, którzy zetkną się z nim po raz pierwszy.
2
Dlaczego to pytanie jest nadal otwarte?
orangepips

Odpowiedzi:

55

Dzielenie wyrażenia, które jest zbyt zagnieżdżone i skomplikowane na prostsze wyrażenia podrzędne przypisane do zmiennych lokalnych, a następnie złożone razem, jest dość powszechną i popularną techniką - zupełnie niezależnie od tego, czy wyrażenia podrzędne i / lub ogólne wyrażenie są prawie każdy inny typ. Przy dobrze dobranych nazwach, gustowna dekompozycja tego rodzaju może zwiększyć czytelność, a dobry kompilator nie powinien mieć problemu z wygenerowaniem kodu odpowiadającego oryginalnemu, skomplikowanemu wyrażeniu.

Niektóre języki, które nie mają pojęcia „przypisania” jako takiego, takie jak Haskell, wprowadzają nawet wyspecjalizowane konstrukcje pozwalające na użycie techniki „nadaj nazwę podwyrażeniu” ( whereklauzula w języku Haskell) - wydaje się, że popularność danej techniki! -)

Alex Martelli
źródło
6
jeśli jest to prostsze i łatwiejsze do odczytania, mogę powiedzieć, że jest całkiem jasna sprawa korzystna :)
djcouchycouch
Zgadzam się. W wielu przypadkach krótki komentarz prawdopodobnie mógłby ujść na sucho, ale nie widzę, jak mogłoby to w rzeczywistości zaszkodzić.
Max E.
16

Użyłem go, chociaż zwykle zawijam logikę boolowską w metodę wielokrotnego użytku (jeśli jest wywoływana z wielu lokalizacji).

Pomaga w czytelności, a kiedy zmienia się logika, wystarczy zmienić ją w jednym miejscu.

Inni zrozumieją to i nie uznają tego za dziwne (z wyjątkiem tych, którzy piszą tylko funkcje tysiąca linii, to znaczy).

Oded
źródło
Dziękuję za odpowiedź! Jeśli chodzi o metody wielokrotnego użytku, ma to inny (niepowiązany) ważny powód do uwzględnienia ... Więc przypuszczam, że moje pytanie naprawdę brzmi, czy należy wziąć pod uwagę jednorazowe wyrażenia boolowskie, gdy nie ma innego powodu niż czytelność. (Co oczywiście samo w sobie jest wystarczającym powodem :)) Dziękuję za zwrócenie uwagi.
froadie
+1 za redukcję zmian w kodzie niezbędnych, gdy zmienia się logika i wyśmiewanie programistów obsługujących funkcje tysiąca linii.
Jeffrey L Whitledge
9

Staram się to robić wszędzie, gdzie to możliwe. Jasne, używasz „dodatkowej linii” kodu, ale jednocześnie opisujesz, dlaczego dokonujesz porównania dwóch wartości.

W twoim przykładzie patrzę na kod i zadaję sobie pytanie „w porządku, dlaczego osoba widząca wartość jest mniejsza niż 0?” W drugiej wyraźnie mi powiesz, że niektóre procesy zostały zakończone, kiedy to nastąpi. W drugim nie zgadnij, jaki był twój zamiar.

Najważniejsze dla mnie jest, gdy widzę metodę: DoSomeMethod(true); Dlaczego jest automatycznie ustawiona na wartość true? Jest dużo bardziej czytelny

bool deleteOnCompletion = true;

DoSomeMethod(deleteOnCompletion);
kemiller2002
źródło
7
Nie lubię parametrów boolowskich właśnie z tego powodu. W końcu otrzymujesz wywołania typu „createOrder (true, false, true, true, false)” i co to oznacza? Wolę używać wyliczeń, więc mówisz coś w stylu „createOrder (Source.MAIL_ORDER, BackOrder.NO, CustomOrder.CUSTOM, PayType.CREDIT)”.
Jay
Ale jeśli pójdziesz za przykładem Kevina, jest to odpowiednik twojego. Jaka to różnica, czy zmienna może przyjmować 2 wartości, czy więcej niż 2?
Mark Ruzon
2
Dzięki Jay's możesz mieć tę przewagę, że w niektórych przypadkach będziesz zdecydowanie bardziej przejrzysty. Na przykład przy użyciu PayType. Gdyby była to wartość logiczna, parametr prawdopodobnie zostałby nazwany isPayTypeCredit. Nie wiesz, jaka jest alternatywa. Dzięki enum możesz wyraźnie zobaczyć, jakie opcje są dostępne w PayType: kredyt, czek, gotówka i wybrać właściwą.
kemiller2002
++ również wyliczenie nie pozwala na przypisanie wartości null, więc kontrola wartości jest dosłownie kompletna, a automatyczna dokumentacja wyliczenia jest na wierzchu
Hardryv
Objective-C i Smalltalk naprawdę rozwiązują ten problem, w Objective-C:[Object createOrderWithSource:YES backOrder:NO custom:YES type:kCreditCard];
Grant Paul
5

Dostarczona próbka:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

Można też przepisać na metody poprawiające czytelność i zachowujące logikę boolowską (jak wskazał Konrad):

if (IsFinished(elementIndex) || IsRepeatedEntry(elementIndex, lastElementIndex)){
   ...
}

...

private bool IsFinished(int elementIndex) {
    return ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
}

private bool IsRepeatedEntry(int elementIndex, int lastElementIndex) {
    return (elementIndex == lastElementIndex);
}

Ma to oczywiście swoją cenę, czyli dwie dodatkowe metody. Jeśli będziesz to robić często, może to uczynić Twój kod bardziej czytelnym, ale Twoje klasy będą mniej przejrzyste. Ale z drugiej strony możesz również przenieść dodatkowe metody do klas pomocniczych.

Prutswonder
źródło
Jeśli masz dużo szumu w kodzie w C #, możesz również skorzystać z klas częściowych i przenieść szum na częściowe, a jeśli ludzie są zainteresowani tym, co sprawdza IsFinished, łatwo jest przejść do.
Chris Marisic,
3

Jedynym sposobem, w jaki mogłem zobaczyć, że to idzie źle, jest to, że fragment logiczny nie ma nazwy, która ma sens, a nazwa została wybrana.

//No clue what the parts might mean.
if(price>0 && (customer.IsAlive || IsDay(Thursday)))

=>

first_condition = price>0
second_condition =customer.IsAlive || IsDay(Thursday)

//I'm still not enlightened.
if(first_condition && second_condition)

Zwracam na to uwagę, ponieważ często tworzy się reguły takie jak „komentuj cały kod”, „używaj nazwanych wartości logicznych dla wszystkich kryteriów if z więcej niż 3 częściami” tylko po to, aby uzyskać komentarze, które są semantycznie puste w następującym sortowaniu

i++; //increment i by adding 1 to i's previous value
MatthewMartin
źródło
2
Źle zgrupowałeś warunki w swoim przykładzie, prawda? „&&” wiąże się mocniej niż „||” w większości języków, które ich używają (skrypty powłoki są wyjątkiem).
Jonathan Leffler
Dodano Parens. Byłby to więc kolejny strajk przeciwko dzieleniu na nazwane zmienne, wprowadza możliwość zmiany grupowania w nieoczywisty sposób.
MatthewMartin
2

Robiąc to

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

usuwasz logikę z mózgu i umieszczasz ją w kodzie. Teraz program wie, co masz na myśli.
Kiedykolwiek nazwać coś, co daje mu fizyczną reprezentację. Istnieje.
Możesz nim manipulować i używać go ponownie.

Możesz nawet zdefiniować cały blok jako predykat:

bool ElementBlahBlah? (elementIndex, lastElementIndex);

i rób więcej rzeczy (później) w tej funkcji.

Nick Dandoulakis
źródło
Co ważniejsze, następny programista, który przyjrzy się kodowi, będzie wiedział, co miałeś na myśli! To świetna praktyka i robię to cały czas.
Chris Thornton,
1
Często następnym deweloperem możesz być Ty, który po kilku miesiącach (a nawet tygodniach) ponownie patrzy na kod i jest zadowolony, że został dobrze udokumentowany.
Louis
2

Jeśli wyrażenie jest złożone, to albo przenoszę je do innej funkcji, która zwraca boolnp., isAnEveningInThePubAGoodIdea(dayOfWeek, sizeOfWorkLoad, amountOfSpareCash)Albo ponownie rozważam kod, aby takie złożone wyrażenie nie było wymagane.

Benedict Cohen
źródło
2

Pamiętaj, że w ten sposób obliczasz więcej niż to konieczne. Ze względu na wyjęcie warunków z kodu, zawsze obliczasz oba z nich (bez zwarcia).

Po to aby:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
   ...
}

Po transformacji staje się:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) |
   (elementIndex == lastElementIndex)){
   ...
}

W większości przypadków nie jest to problem, ale nadal w niektórych może oznaczać gorszą wydajność lub inne problemy, np. Gdy w drugim wyrażeniu założysz, że pierwszy zawiódł.

Konrad Garus
źródło
To prawda - zauważyłem to teraz, gdy wróciłem do części mojego kodu, aby zrefaktoryzować kilka instrukcji if przy użyciu tej metody. Była tam instrukcja if korzystająca z oceny zwarcia z && (druga część zgłasza NPE, jeśli pierwsza część jest fałszywa), a mój kod nie powiódł się podczas refaktoryzacji (ponieważ zawsze oceniał oba i zapisywał je w wartości logicznej zmienne). Słuszna uwaga, dzięki! Ale zastanawiałem się, próbując tego - czy istnieje sposób, aby zapisać logikę w zmiennej i opóźnić ocenę do rzeczywistej instrukcji if?
froadie
2
Właściwie wątpię, czy kompilator to rozwiąże. Jeśli drugie wywołanie nie jest skuteczne, zwykle jest to spowodowane wywołaniem jakiejś funkcji, a AFAIK żaden kompilator nie próbuje określić, czy wywoływana funkcja nie ma skutków ubocznych.
JS
Możesz zagnieżdżać IF i nie wykonywać późniejszych obliczeń, chyba że pierwszy test jest niewystarczający, aby zdecydować, czy kontynuować.
Jay
@froadie Niektóre języki, takie jak Kotlin (i wkrótce Dart), dopuszczają „leniwe” zmienne, które będą obliczane tylko wtedy, gdy są używane. Alternatywnie, umieszczenie logiki w funkcji zamiast w zmiennej będzie miało ten sam efekt tutaj. Wiesz, na wypadek, gdybyś chciał wiedzieć 10 lat później.
hacker1024
2

Myślę, że lepiej jest tworzyć funkcje / metody zamiast zmiennych tymczasowych. W ten sposób zwiększa się czytelność również dlatego, że metody stają się krótsze. Książka Martina Fowlera Refactoring zawiera dobre rady dotyczące poprawy jakości kodu. Refaktoryzacje związane z konkretnym przykładem nazywają się „Zastąp temp. Zapytaniem” i „Wyodrębnij metodę”.

mkj
źródło
2
Czy chcesz powiedzieć, że zagracając przestrzeń klasy wieloma jednorazowymi funkcjami, zwiększasz czytelność? Proszę wytłumacz.
Zano
To zawsze kompromis. Poprawi się czytelność oryginalnej funkcji. Jeśli oryginalna funkcja jest krótka, może nie być tego warta.
mkj
Myślę, że także „zaśmiecanie przestrzeni klas” zależy od używanego języka i sposobu podziału kodu.
mkj
2

Osobiście uważam, że to świetna praktyka. Jego wpływ na wykonanie kodu jest minimalny, ale przejrzystość, jaką może zapewnić, jeśli jest właściwie używana, jest nieoceniona, jeśli chodzi o późniejszą konserwację kodu.

GrowlingDog
źródło
1

jeśli metoda wymaga powiadomienia o sukcesie: (przykłady w c #) Lubię używać

bool success = false;

zacząć. kod nie działa, dopóki nie zmienię go na:

success = true;

potem na końcu:

return success;
Chris Hayes
źródło
0

Myślę, że to zależy od stylu preferowanego przez Ciebie / Twój zespół. Refaktoryzacja „Wprowadź zmienną” może być przydatna, ale czasami nie :)

I nie zgadzam się z Kevinem w jego poprzednim poście. Jego przykład, jak przypuszczam, nadający się do użytku w przypadku, gdy wprowadzona zmienna może zostać zmieniona, ale wprowadzenie jej tylko dla jednego statycznego boolean jest bezużyteczne, ponieważ w deklaracji metody mamy nazwę parametru, więc po co powielać ją w kodzie?

na przykład:

void DoSomeMethod(boolean needDelete) { ... }

// useful
boolean deleteOnCompletion = true;
if ( someCondition ) {
    deleteOnCompletion = false;
}
DoSomeMethod(deleteOnCompletion);

// useless
boolean shouldNotDelete = false;
DoSomeMethod(shouldNotDelete);
dchekmarev
źródło
0

Z doświadczenia wiem, że często wracałem do starych scenariuszy i zastanawiałem się „o czym ja do cholery myślałem wtedy?”. Na przykład:

Math.p = function Math_p(a) {
    var r = 1, b = [], m = Math;
    a = m.js.copy(arguments);
    while (a.length) {
        b = b.concat(a.shift());
    }
    while (b.length) {
        r *= b.shift();
    }
    return r;
};

który nie jest tak intuicyjny jak:

/**
 * An extension to the Math object that accepts Arrays or Numbers
 * as an argument and returns the product of all numbers.
 * @param(Array) a A Number or an Array of numbers.
 * @return(Number) Returns the product of all numbers.
 */
Math.product = function Math_product(a) {
    var product = 1, numbers = [];
    a = argumentsToArray(arguments);
    while (a.length) {
        numbers = numbers.concat(a.shift());
    }
    while (numbers.length) {
        product *= numbers.shift();
    }
    return product;
};
rolandog
źródło
-1. Szczerze mówiąc, ma to trochę wspólnego z pierwotnym pytaniem. Pytanie dotyczy innej, bardzo konkretnej rzeczy, a nie ogólnie tego, jak pisać dobry kod.
P Shved
0

Rzadko tworzę oddzielne zmienne. To, co robię, gdy testy stają się skomplikowane, to zagnieżdżanie IF i dodawanie komentarzy. Lubić

boolean processElement=false;
if (elementIndex < 0) // Do we have a valid element?
{
  processElement=true;
}
else if (elementIndex==lastElementIndex) // Is it the one we want?
{
  processElement=true;
}
if (processElement)
...

Przyznaną wadą tej techniki jest to, że następny programista, który się pojawi, może zmienić logikę, ale nie zadaje sobie trudu, aby aktualizować komentarze. Wydaje mi się, że jest to ogólny problem, ale wiele razy widziałem komentarz mówiący „Sprawdź identyfikator klienta”, a następny wiersz dotyczy sprawdzenia numeru części lub czegoś podobnego i zastanawiam się, gdzie klient id wchodzi.

Sójka
źródło