Poprawny komentarz do argumentów funkcji boolowskich, które są „fałszywe”?

19

Z niektórych projektów typu open source zebrałem następujący styl kodowania

void someFunction(bool forget);

void ourFunction() {
  someFunction(false /* forget */);
}    

Zawsze mam wątpliwości, co falsetu oznacza. Czy oznacza to „zapomnieć”, czy też „zapomnieć” odnosi się do odpowiedniego parametru (jak w powyższym przypadku), a „fałsz” ma go zanegować?

Jaki styl jest używany najczęściej i jaki jest najlepszy sposób (lub kilka lepszych sposobów), aby uniknąć dwuznaczności?

Johannes Schaub - litb
źródło
38
używaj wyliczeń (nawet jeśli są tylko 2 opcje) zamiast
booli
21
Niektóre języki obsługują nazwane argumenty. W takich językach można użyćsomeFunction(forget: true);
Brian
3
Czuję się zobowiązany do przedstawienia argumentów Martina Fowlera na temat argumentów flagowych (mówi również o seterach boolowskich). Ogólnie staraj się ich unikać.
FGreg
3
Aby pochwalić to, co oczywiste, komentarze mogą kłamać. Tak więc jest zawsze lepiej, aby swój własny kod dokumentowania, ponieważ niektóre zmieni truesię falsei nie zaktualizować komentarz. Jeśli nie możesz zmienić interfejsu API, najlepszym sposobem na skomentowanie tego jestsomeFunction( false /* true=forget, false=remember */)
Mark Lakata,
1
@ Bakuriu - właściwie prawdopodobnie nadal mam publiczny interfejs API, który ma dwie osobne metody ( sortAscendingi sortDescending, lub podobne). Teraz wewnątrz mogą obaj wywołać tę samą metodę prywatną, która może mieć tego rodzaju parametr. Właściwie, jeśli język to obsługuje, prawdopodobnie przekazałbym funkcję lambda, która zawierała kierunek sortowania ...
Clockwork-Muse

Odpowiedzi:

33

W opublikowanym przez Ciebie przykładowym kodzie wygląda to jak forgetargument flagi. (Nie mogę być pewien, ponieważ funkcja jest czysto hipotetyczna).

Argumenty flagi są zapachem kodu. Wskazują, że funkcja wykonuje więcej niż jedną rzecz, a dobra funkcja powinna robić tylko jedną rzecz.

Aby uniknąć argumentu flagi, podziel funkcję na dwie funkcje, które wyjaśniają różnicę w nazwie funkcji.

Zgłoś argument

serveIceCream(bool lowFat)

Brak argumentu flagi

serveTraditionalIceCream()
serveLowFatIceCream()

edycja: Idealnie, nie musisz wcale obchodzić funkcji z parametrem flag. Są przypadki podobne do tego, co Fowler nazywa splątaną implementacją, w której całkowite oddzielenie funkcji tworzy zduplikowany kod. Jednak im wyższa cykliczna złożoność sparametryzowanej funkcji, tym silniejszy jest argument za jej pozbyciem się.


To tylko przeczucie, ale parametr o nazwie forgetbrzmi jak zazdrość o funkcję. Dlaczego dzwoniący miałby mówić innemu obiektowi, aby o czymś zapomniał? Może występować większy problem projektowy.

Aaron Kurtzhals
źródło
4
+17, hełm założony. Jak wyglądają ciała serveTraditionalIceCreami serveLowFatIceCreamjak wyglądają? Mam enum z 14 rodzajami lodów.
JohnMark13,
13
W przypadku metod publicznych ta konwencja jest dobra, ale jak wspomina JohnMark, druga połowa SRP to „dobra metoda powinna być jedyną metodą, która robi to, co robi”. Widzę, że istnieje N + 1 wariantów tej metody; N publiczny bez parametrów, z których wszystkie wywołują jedną metodę prywatną z parametrem, którego próbujesz uniknąć. W pewnym momencie po prostu poddajesz się i wystawiasz ten cholerny parametr. Zapachy kodu niekoniecznie oznaczają, że kod powinien zostać ponownie przetworzony; to tylko rzeczy, które zasługują na drugie spojrzenie w przeglądzie kodu.
KeithS,
@Aaron Przez „zazdrość”, co miałeś na myśli?
Geek
27

Słowami laika:

  • false jest dosłowne.
  • przekazujesz literał false
  • mówisz, someFunctionżeby nie zapomnieć
  • mówisz, someFunctionże parametr zapomnij tofalse
  • mówisz someFunctionpamiętać

Moim zdaniem byłoby lepiej, gdyby funkcja była taka:

void someFunction(bool remember);

możesz to nazwać

void ourFunction() {
  someFunction(true);
} 

lub zachowaj starą nazwę, ale zmień funkcję opakowania na

void ourFunctionWithRemember() {
  someFunction(false);
} 

EDYTOWAĆ:

Jak stwierdził @Vorac, zawsze staraj się używać pozytywnych słów. Podwójna negacja jest myląca.

Tulains Córdova
źródło
15
+1 za pomysł, aby zawsze starać się używać pozytywnych słów. Podwójna negacja jest myląca.
Vorac,
1
Uzgodniony, świetny pomysł. Mówienie systemowi, aby nie robił czegoś, jest mylące. Jeśli akceptujesz parametr boolowski, wyraż go pozytywnie.
Brandon,
W większości przypadków, myślę, że też chcą być bardziej szczegółowe niż remember, chyba że nazwa funkcji wykonane sens remember bardzo oczywiste. rememberToCleanUp* or *persistlub coś.
itsbruce
14

Parametr może być dobrze nazwany; trudno powiedzieć, nie znając nazwy funkcji. Zakładam, że komentarz został napisany przez oryginalnego autora funkcji, i to było przypomnienie o co przechodząc falsedo someFunctionśrodka, ale dla każdego, kto idzie potem, że to trochę niejasne na pierwszy rzut oka.

Użycie dodatniej nazwy zmiennej (sugerowanej w Code Complete ) może być najprostszą zmianą, która ułatwiłaby odczytanie tego fragmentu, np.

void someFunction(boolean remember);

następnie ourFunctionstaje się:

void ourFunction() {
    someFunction(true /* remember */);
}

Jednak użycie wyliczenia sprawia, że ​​wywołanie funkcji jest jeszcze łatwiejsze do zrozumienia, kosztem niektórych kodów pomocniczych:

public enum RememberFoo {
    REMEMBER,
    FORGET
}

...

void someFunction(RememberFoo remember);

...

void ourFunction() {
    someFunction(RememberFoo.REMEMBER);
}

Jeśli nie możesz zmienić podpisu z someFunctionjakiegokolwiek powodu, użycie zmiennej tymczasowej ułatwia również odczytanie kodu, coś w rodzaju uproszczenia warunków warunkowych poprzez wprowadzenie zmiennej bez żadnego innego powodu niż ułatwienie parsowania kodu przez ludzi .

void someFunction(boolean remember);

...

void ourFunction() {
    boolean remember = false;
    someFunction(remember);
}
Mike Partridge
źródło
1
Ustawienie wartości remembertrue oznacza zapomnienie (w twoim przykładzie someFunction(true /* forget */);)?
Brian
2
To enumzdecydowanie najlepsze rozwiązanie. Tylko dlatego, że typ może być reprezentowany jako - booltj. Są izomorficzne - nie oznacza to, że powinien być reprezentowany jako taki. Ten sam argument dotyczy, stringa nawet int.
Jon Purdy,
10

Zmień nazwę zmiennej, aby wartość bool miała sens.

To milion razy lepsze niż dodawanie komentarza w celu wyjaśnienia argumentów funkcji, ponieważ nazwa jest niejednoznaczna.

kraina krańca
źródło
3
To nie odpowiada na pytanie. Wszystkie rzeczy są oczywiste, gdy metoda jest zdefiniowana i wywołana w tym samym pliku w 4 liniach od siebie. Ale co jeśli w tej chwili widzisz tylko dzwoniącego? Co jeśli ma wiele boolanów? Czasami prosty komentarz w linii przechodzi długą drogę.
Brandon,
@Brandon To nie jest argument przeciwko wywołaniu boolean doNotPersist (lub, lepiej, Persist). Nazywanie tego „zapomnieniem” bez mówienia o tym, co należy zapomnieć, jest szczerze mówiąc nieprzydatne. Aha, i metoda, która bierze kilka booleanów jako opcję, śmierdzi do wysokiego nieba.
itsbruce
5

Utwórz lokalny boolean o bardziej opisowej nazwie i przypisz mu wartość. W ten sposób znaczenie będzie bardziej wyraźne.

void ourFunction() {
    bool takeAction = false;  /* false means to forget */
    someFunction( takeAction );
}    

Jeśli nie możesz zmienić nazwy zmiennej, komentarz powinien być nieco bardziej wyrazisty:

void ourFunction() {
    /* false means that the method should forget what was requested */
    someFunction( false );
}    

źródło
1
Zdecydowanie dobra rada, ale nie sądzę, aby odnosiła się do problemu, którym /* forget */jest komentarz, który polega na tym, że bez deklaracji funkcji tuż przed tobą może być trudno zapamiętać, co jest ustawione false. (Dlatego uważam, że rada @ Esailii dotycząca dodania wyliczenia jest lepsza i dlatego uwielbiam języki, które pozwalają na nazwane parametry.)
Gort the Robot
@StevenBurnap - dzięki! Masz rację, że moja stara odpowiedź nie była wystarczająco jasna w odpowiedzi na pytanie PO. Zredagowałem go, aby był bardziej przejrzysty.
3

Jest dobry artykuł, który wspomina o tej dokładnej sytuacji, ponieważ odnosi się do interfejsów API w stylu Qt. Tam nazywa się to Boolean Parameter Trap i warto ją przeczytać.

Jego istotą jest:

  1. Lepsze jest przeciążenie funkcji, aby bool nie był potrzebny
  2. Używanie wyliczenia (jak sugeruje Esailija) jest najlepsze
Steven Evers
źródło
2

To dziwny komentarz.

Z punktu widzenia kompilatora someFunction(false /* forget */);jest faktycznie someFunction(false);(komentarz jest usuwany). Zatem wszystko, co robi linia, to wywołanie someFunctionz pierwszym (i jedynym) argumentem ustawionym na false.

/* forget */to tylko nazwa parametru. Prawdopodobnie jest to nic innego jak szybkie (i brudne) przypomnienie, które tak naprawdę nie musi tam być. Wystarczy użyć mniej dwuznacznej nazwy parametru, a komentarz w ogóle nie będzie potrzebny.

użytkownik2590712
źródło
1

Jedną z rad Czystego kodu jest zminimalizowanie liczby niepotrzebnych komentarzy 1 (ponieważ mają one tendencję do gnicia) oraz prawidłowe nazwanie funkcji i metod.

Następnie usunę komentarz. W końcu nowoczesne środowiska IDE (takie jak zaćmienie) wyskakują z kodem po najechaniu myszką na funkcję. Widzenie kodu powinno usunąć niejasności.


1 Komentowanie złożonego kodu jest w porządku.

BЈовић
źródło
btw Kto powiedział coś takiego: „najgorszym problemem programisty jest to, jak nazwać zmienną i przesunąć o jeden”?
BЈовић
4
Prawdopodobnie szukasz źródła martinfowler.com/bliki/TwoHardThings.html . Słyszałem, że dostosowałem się do: „W informatyce są tylko dwie trudne rzeczy: unieważnienie pamięci podręcznej, nadawanie nazw i wyłączanie przez jeden błąd”.
1

Aby pochwalić to, co oczywiste, komentarze mogą kłamać. Dlatego zawsze lepiej jest samokontrować kod bez uciekania się do komentarzy w celu wyjaśnienia, ponieważ pewna osoba (być może ty) zmieni się truenafalse i nie zaktualizować komentarz.

Jeśli nie możesz zmienić interfejsu API, skorzystaj z 2 opcji

  • Zmień komentarz, aby zawsze był prawdziwy, niezależnie od kodu. Jeśli zadzwonisz tylko raz, jest to dobre rozwiązanie, ponieważ utrzymuje dokumentację lokalnie.
     someFunction (false / * true = zapomnij, false = zapamiętaj * /); `
  • Użyj #defines, zwłaszcza jeśli wywołujesz go więcej niż jeden raz.
     # zdefiniować ZAPOMNIJ true
     # zdefiniować PAMIĘTAJ false
     someFunction (PAMIĘTAJ);
Mark Lakata
źródło
1

Podoba mi się odpowiedź na temat tego, aby komentarz zawsze był prawdziwy , ale choć dobrze myślę, że pomija podstawowy problem z tym kodem - nazywa się go dosłownie.

Powinieneś unikać używania literałów podczas wywoływania metod. Zmienne lokalne, parametry opcjonalne, parametry nazwane, wyliczenia - to, jak najlepiej ich uniknąć, będzie zależeć od języka i dostępnych opcji, ale staraj się ich unikać. Literały mają wartości, ale nie mają znaczenia.

jmoreno
źródło
-1

W C # użyłem nazwanych parametrów, aby to wyjaśnić

someFunction(forget: false);

Lub enum:

enum Memory { Remember, Forget };

someFunction(Memory.Forget);

Lub przeciążenia:

someFunctionForget();

Lub polimorfizm`:

var foo = new Elephantine();

foo.someFunction();
Jay Bazuzi
źródło
-2

Nazewnictwo powinno zawsze eliminować dwuznaczność dla boolean. Zawsze nazywam boolean coś takiego jak „isThis” lub „shouldDoThat”, na przykład:

void printTree(Tree tree, bool shouldPrintPretty){ ... }

i tak dalej. Ale gdy odwołujesz się do kodu innej osoby, najlepiej pozostaw komentarz podczas przekazywania wartości.

dchhetri
źródło
Jak to odpowiada na zadane pytanie?
komar
@gnat Odpowiadałem na jego pytanie dotyczące rozwiązywania dwuznaczności za pomocą parametrów boolowskich. Może źle odczytałem jego pytanie.
dchhetri