Według Czy niewłaściwe jest używanie parametru boolowskiego do określania zachowania? , Wiem, jak ważne jest unikanie używania parametrów boolowskich do określania zachowania, np .:
orginalna wersja
public void setState(boolean flag){
if(flag){
a();
}else{
b();
}
c();
}
Nowa wersja:
public void setStateTrue(){
a();
c();
}
public void setStateFalse(){
b();
c();
}
Ale co z przypadkiem, w którym parametr boolowski jest używany do określania wartości zamiast zachowań? na przykład:
public void setHint(boolean isHintOn){
this.layer1.visible=isHintOn;
this.layer2.visible=!isHintOn;
this.layer3.visible=isHintOn;
}
Próbuję wyeliminować flagę isHintOn i utworzyć 2 osobne funkcje:
public void setHintOn(){
this.layer1.visible=true;
this.layer2.visible=false;
this.layer3.visible=true;
}
public void setHintOff(){
this.layer1.visible=false;
this.layer2.visible=true;
this.layer3.visible=false;
}
ale zmodyfikowana wersja wydaje się mniej konserwowalna, ponieważ:
ma więcej kodów niż oryginalna wersja
nie może wyraźnie pokazać, że widoczność warstwy 2 jest przeciwna do opcji podpowiedzi
kiedy dodaje się nową warstwę (np .: layer4), muszę ją dodać
this.layer4.visible=false;
i
this.layer4.visible=true;
na setHintOn () i setHintOff () oddzielnie
Więc moje pytanie brzmi: jeśli parametr boolowski jest używany tylko do określania wartości, ale nie zachowań (np .: brak parametru if-else tego parametru), czy nadal zaleca się wyeliminowanie tego parametru boolowskiego?
setHint(boolean isHintOn)
jako metodę prywatną i dodaj publicsetHintOn
orazsetHintOff
metody, które odpowiednio wywołująsetHint(true)
isetHint(false)
.setHint(true|false)
. Ziemniak Potahto Przynajmniej użyj czegoś takiego jaksetHint
iunsetHint
.is
na początku.isValid
itd. Więc po co to zmieniać na dwa słowa? Poza tym „bardziej naturalny” jest w oku patrzącego. Jeśli chcesz wymówić to zdanie w języku angielskim, bardziej naturalne byłoby dla mnie „jeśli wskazówka jest włączona” z „the” schowanym.Odpowiedzi:
Projektowanie interfejsu API powinno koncentrować się na tym, co jest najbardziej użyteczne dla klienta interfejsu API, od strony wywołującej .
Na przykład, jeśli ten nowy interfejs API wymaga od dzwoniącego regularnego pisania takiego kodu
to powinno być oczywiste, że unikanie parametru jest gorsze niż posiadanie interfejsu API, który umożliwia programowi wywołującemu pisanie
Poprzednia wersja powoduje tylko problem, który należy rozwiązać po stronie wywołującej (i prawdopodobnie więcej niż raz). Nie zwiększa to czytelności ani łatwości konserwacji.
Strona implementacyjna nie powinna jednak decydować o tym, jak wygląda publiczny interfejs API. Jeśli funkcja taka jak
setHint
parametr wymaga mniej kodu podczas implementacji, ale interfejs API w kategoriachsetHintOn
/setHintOff
wygląda na łatwiejszy w obsłudze dla klienta, można go zaimplementować w ten sposób:Tak więc, chociaż publiczny interfejs API nie ma parametru logicznego, nie ma tutaj zduplikowanej logiki, więc tylko jedno miejsce można zmienić, gdy pojawi się nowe wymaganie (jak w przykładzie pytania).
Działa to również na odwrót: jeśli
setState
powyższa metoda wymaga przełączania między dwoma różnymi fragmentami kodu, fragmenty kodu można przekształcić w dwie różne metody prywatne. Zatem IMHO nie ma sensu szukać kryterium decydującego między „jednym parametrem / jedną metodą” a „zerowymi parametrami / dwiema metodami”, patrząc na elementy wewnętrzne. Spójrz jednak na sposób, w jaki chciałbyś zobaczyć API w roli jego konsumenta.W razie wątpliwości spróbuj użyć „rozwoju opartego na testach” (TDD), co zmusi Cię do myślenia o publicznym interfejsie API i sposobie jego używania.
źródło
SetLoanFacility(bool enabled)
, ponieważ udzielenie pożyczki może nie być łatwe jej ponowne zabranie, a dwie opcje mogą obejmować zupełnie inną logikę - i chciałbyś przejść do oddzielnego Utwórz / Usuń metody.Toggle()
to nie jest odpowiednia funkcja do zapewnienia. To jest cały punkt; jeśli rozmówcy zależy tylko na „zmianie”, a nie „tym, co się skończy jako”, toToggle()
jest opcja, która pozwala uniknąć dodatkowych sprawdzeń i podejmowania decyzji. Nie nazwałbym tego WSPÓLNYM przypadkiem, ani nie zaleciłbym udostępnienia go bez uzasadnionego powodu, ale jeśli użytkownik potrzebuje przełącznika, dałbym mu przełączenie.Martin Fowler cytuje Kent Beck, zalecając osobne
setOn()
setOff()
metody , ale także mówi, że nie należy tego uważać za nienaruszalne:Kolejnym zaleceniem jest użyć flagi wyliczone wartości lub wpisać dać
true
ifalse
lepiej, nazwy kontekst specyficzne. W przykładzie,showHint
ahideHint
może być lepiej.źródło
-[NSView setNeedsDisplay:]
metoda, w której można przekazać,YES
czy widok powinien zostać przerysowany, aNO
jeśli nie. Prawie nigdy nie musisz tego mówić, więc UIKit po prostu nie-[UIView setNeedsDisplay]
ma parametru. Nie ma odpowiedniej-setDoesNotNeedDisplay
metody.bool isPremium
flagi w jego przykładzie, ale użyłbym enum (BookingType bookingType
) do parametryzacji tej samej metody, chyba że logika dla każdej rezerwacji była inna. „Splątana logika”, do której odnosi się Fowler, jest często pożądana, jeśli chce się zobaczyć, jaka jest różnica między tymi dwoma trybami. A jeśli są one radykalnie różne, odsłoniłbym sparametryzowaną metodę zewnętrznie i zaimplementowałem osobne metody wewnętrznie.Myślę, że łączysz dwie rzeczy w swoim poście, API i implementacji. W obu przypadkach nie sądzę, aby istniała silna zasada, z której można korzystać cały czas, ale należy rozważyć te dwie rzeczy niezależnie (w miarę możliwości).
Zacznijmy od API, zarówno:
i:
są ważnymi alternatywami w zależności od tego, co twój obiekt ma do zaoferowania i jak klienci będą korzystać z interfejsu API. Jak zauważył Doc, jeśli Twoi użytkownicy mają już zmienną logiczną (z kontrolki interfejsu użytkownika, akcji użytkownika, interfejsu zewnętrznego, API itp.), Pierwsza opcja ma większy sens, w przeciwnym razie wymuszasz dodatkową instrukcję if w kodzie klienta . Jeśli jednak na przykład zmienisz wskazówkę na true podczas rozpoczynania procesu i na false na końcu, pierwsza opcja daje coś takiego:
podczas gdy druga opcja daje to:
który IMO jest znacznie bardziej czytelny, więc w tym przypadku wybiorę drugą opcję. Oczywiście nic nie stoi na przeszkodzie, aby zaoferować obie opcje (lub więcej, możesz użyć wyliczenia, jak powiedział Graham, jeśli ma to na przykład większy sens).
Chodzi o to, że powinieneś wybrać swój interfejs API na podstawie tego, co powinien zrobić obiekt i jak klienci będą go używać, a nie na podstawie tego, jak zamierzasz go wdrożyć.
Następnie musisz wybrać sposób implementacji publicznego interfejsu API. Powiedzmy, że wybraliśmy metody
setHintOn
isetHintOff
nasz publiczny interfejs API i mają one wspólną logikę, jak w twoim przykładzie. Możesz łatwo wyodrębnić tę logikę za pomocą prywatnej metody (kod skopiowany z Doc):I odwrotnie, powiedzmy, że wybraliśmy
setHint(boolean isHintOn)
nasz interfejs API, ale odwróćmy twój przykład, z jakiegokolwiek powodu ustawienie podpowiedzi On jest zupełnie inne niż ustawienie jej na Wył. W takim przypadku możemy go zaimplementować w następujący sposób:Lub nawet:
Chodzi o to, że w obu przypadkach najpierw wybraliśmy nasz publiczny interfejs API, a następnie dostosowaliśmy naszą implementację do wybranego interfejsu API (i ograniczeń, które mamy), a nie na odwrót.
Nawiasem mówiąc, myślę, że to samo dotyczy posta, który podłączyłeś, używając parametru boolean do określenia zachowania, tj. Powinieneś zdecydować na podstawie konkretnego przypadku użycia, a nie jakiejś twardej reguły (chociaż w tym konkretnym przypadku zwykle jest to właściwe rozbija go na wiele funkcji).
źródło
begin
iend
lub synonimów, aby jasno powiedzieć, że tak właśnie robią, i sugerować, że początek musi mieć zakończenie i odwrotnie.using
bloki w C #,with
menedżery kontekstu instrukcji w Pythonie, przekazywanie ciało jak lambda lub na żądanie obiektu (na przykład składni bloków Ruby), itpPo pierwsze: kod nie jest automatycznie mniej konserwowalny, tylko dlatego, że jest nieco dłuższy. Najważniejsza jest przejrzystość.
Teraz, jeśli tak naprawdę masz do czynienia z danymi, to ustawiasz właściwość boolean. W takim przypadku możesz po prostu zapisać tę wartość bezpośrednio i uzyskać widoczność warstwy, tj
(Pozwoliłem nadać warstwom rzeczywiste nazwy - jeśli nie masz tego w swoim oryginalnym kodzie, zacznę od tego)
To wciąż pozostawia pytanie, czy zastosować
setHintVisibility(bool)
metodę. Osobiście poleciłbym zastąpienie go metodąshowHint()
ihideHint()
- oba będą naprawdę proste i nie będziesz musiał ich zmieniać podczas dodawania warstw. Nie jest to jednak jednoznaczne określenie „dobrze / źle”.Teraz, jeśli wywołanie funkcji powinno faktycznie zmienić widoczność tych warstw, to faktycznie masz zachowanie. W takim przypadku zdecydowanie zaleciłbym osobne metody.
źródło
showHint
vssetHintVisibility
). Wspomniałem o nowej warstwie tylko dlatego, że OP się o nią martwiła. Ponadto, musimy tylko dodać jedną nową metodę:isLayer4Visible
.showHint
ihideHint
po prostu ustawisHintVisible
atrybut na true / false, a to się nie zmienia.Parametry logiczne są poprawne w drugim przykładzie. Jak już się zorientowałeś, parametry boolowskie same w sobie nie stanowią problemu. Problematyczne jest zachowanie przełączania na podstawie flagi.
Pierwszy przykład jest jednak problematyczny, ponieważ nazewnictwo oznacza metodę ustawiającą, ale implementacje wydają się być czymś innym. Masz więc antipattern zmieniający zachowanie i wprowadzającą w błąd metodę. Ale jeśli metoda jest w rzeczywistości zwykłym ustawiaczem (bez przełączania zachowań), nie ma z tym problemu
setState(boolean)
. Posiadanie dwóch metodsetStateTrue()
isetStateFalse()
po prostu niepotrzebnie komplikuje rzeczy bez korzyści.źródło
Innym sposobem rozwiązania tego problemu jest wprowadzenie obiektu reprezentującego każdą podpowiedź, który będzie odpowiedzialny za określenie wartości logicznych powiązanych z tą podpowiedź. W ten sposób możesz dodawać nowe permutacje zamiast po prostu mieć dwa stany logiczne.
Na przykład w Javie możesz:
A wtedy twój kod dzwoniącego wyglądałby tak:
Twój kod implementacyjny wyglądałby tak:
Dzięki temu kod implementacyjny i kod dzwoniącego są zwięzłe, w zamian za zdefiniowanie nowego typu danych, który wyraźnie odwzorowuje silnie typowane, nazwane intencje na odpowiednie zestawy stanów. Myślę, że tak jest lepiej.
źródło
Kiedy mam wątpliwości co do takich rzeczy. Lubię sobie wyobrazić, jak wyglądałby ślad stosu.
Przez wiele lat pracowałem nad projektem PHP, który używał tej samej funkcji co setter i getter . Jeśli wartość minę zero , zwróci wartość, w przeciwnym razie ustaw ją. To było okropne w pracy .
Oto przykład śledzenia stosu:
Nie miałeś pojęcia o stanie wewnętrznym, co utrudniło debugowanie. Istnieje wiele innych negatywnych skutków ubocznych, ale spróbuj zobaczyć, że w pełnej nazwie funkcji i przejrzystości jest wartość, gdy funkcje wykonują pojedyncze działanie.
Teraz wyobrażam sobie pracę ze śledzeniem stosu dla funkcji, która ma zachowanie A lub B w oparciu o wartość logiczną.
To mylące, jeśli mnie pytasz. Te same
setVisible
linie dają dwie różne ścieżki śledzenia.Wróćmy do twojego pytania. Spróbuj wyobrazić sobie, jak będzie wyglądał ślad stosu, jak komunikuje się z osobą, co się dzieje, i zadaj sobie pytanie, czy pomagasz przyszłej osobie w debugowaniu kodu.
Oto kilka porad:
Czasami kod wygląda pod nadmiernym mikroskopem, ale kiedy cofniesz się do większego obrazu, minimalistyczne podejście sprawi, że zniknie. Jeśli potrzebujesz go wyróżnić, abyś mógł go utrzymać. Dodanie wielu małych funkcji może wydawać się zbyt szczegółowe, ale poprawia łatwość konserwacji, gdy jest szeroko stosowane w szerszym kontekście.
źródło
setVisible
śledzeniu stosu jest to, że wywoływaniesetVisible(true)
wydaje się wywoływać wywołanie dosetVisible(false)
(lub odwrotnie, w zależności od tego, w jaki sposób wywołano ten ślad).W prawie każdym przypadku, gdy przekazujesz
boolean
parametr do metody jako flagę, aby zmienić zachowanie czegoś, powinieneś rozważyć bardziej wyraźny i bezpieczny sposób, aby to zrobić.Jeśli nie robisz nic więcej niż używasz tego,
Enum
który reprezentuje stan, poprawiłeś zrozumienie swojego kodu.W tym przykładzie użyto
Node
klasy zJavaFX
:jest zawsze lepszy niż, jak na wielu
JavaFX
obiektach:ale myślę
.setVisible()
i.setHidden()
są najlepszym rozwiązaniem dla sytuacji, w której flaga jest,boolean
ponieważ jest najbardziej wyraźna i najmniej szczegółowa.w przypadku wielu opcji wybór jest jeszcze ważniejszy, aby zrobić to w ten sposób.
EnumSet
istnieje właśnie z tego powodu.źródło
Przy podejmowaniu decyzji między podejściem do jakiegoś interfejsu, który przechodzi parametr (boolowski) a metody przeciążone bez tego parametru, spójrz na klientów zużywających się.
Jeśli wszystkie zastosowania przekazywałyby wartości stałe (np. Prawda, fałsz), to przemawia za tym przeciążenie.
Jeśli wszystkie zastosowania przekazałyby wartość zmiennej, to przemawia za tym metoda z podejściem parametrycznym.
Jeśli żadna z tych skrajności nie ma zastosowania, oznacza to, że istnieje mieszanka zastosowań klienta, więc musisz wybrać, czy wspierać obie formularze, czy też dostosować jedną kategorię klientów do drugiego (co jest dla nich bardziej nienaturalne).
źródło
Istnieją dwa czynniki do zaprojektowania:
Nie należy ich łączyć.
Zupełnie dobrze jest:
Jako taki, każdy argument, który próbuje zrównoważyć koszty / korzyści projektu interfejsu API z kosztami / korzyściami projektu wdrożenia, jest wątpliwy i należy go dokładnie zbadać.
Po stronie API
Jako programista ogólnie preferuję programowalne interfejsy API. Kod jest znacznie wyraźniejszy, gdy mogę przekazać wartość, niż gdy potrzebuję instrukcji
if
/switch
wartości, aby zdecydować, którą funkcję wywołać.To ostatnie może być konieczne, jeśli każda funkcja oczekuje różnych argumentów.
W takim przypadku jedna metoda
setState(type value)
wydaje się lepsza.Jednakże , nie ma nic gorszego niż Nameless
true
,false
,2
, etc ... te wartości nie mają znaczenia magiczne na własną rękę. Unikaj prymitywnej obsesji i używaj silnego pisania.Tak więc, z POV API, chcę:
setState(State state)
.Po stronie wdrożenia
Polecam robić wszystko, co jest łatwiejsze.
Jeśli metoda jest prosta, najlepiej trzymać ją razem. Jeśli przepływ kontrolny jest zawiły, najlepiej jest go rozdzielić na wiele metod, z których każda dotyczy podklucza lub kroku potoku.
Na koniec rozważ grupowanie .
W twoim przykładzie (z dodaną białą spacją dla czytelności):
Dlaczego
layer2
przełamywanie trendu? Czy to funkcja, czy błąd?Czy to możliwe, aby mieć 2 listy
[layer1, layer3]
i[layer2]
, z wyraźną nazwą wskazującą powody są one zgrupowane razem, a następnie iteracyjne nad tymi listami.Na przykład:
Kod mówi sam za siebie, jasne jest, dlaczego istnieją dwie grupy, a ich zachowanie się różni.
źródło
Oddzielnie od pytania
setOn() + setOff()
vsset(flag)
zastanowię się, czy typ boolowski jest tutaj najlepszy. Czy jesteś pewien, że nigdy nie będzie trzeciej opcji?Może warto rozważyć wyliczenie zamiast wartości logicznej. Oprócz umożliwienia rozszerzalności, utrudnia to również uzyskanie niewłaściwej wartości logicznej, np .:
vs
Dzięki wyliczeniu będzie znacznie łatwiej rozszerzyć, gdy ktoś zdecyduje, że chce opcji „w razie potrzeby”:
vs
źródło
Proponuję ponownie ocenić tę wiedzę.
Przede wszystkim nie widzę wniosku, który proponujecie w połączonym ze sobą pytaniu SE. Mówią głównie o przekazywaniu parametru przez kilka etapów wywołań metod, gdzie jest on oceniany bardzo daleko w dół łańcucha.
W twoim przykładzie oceniasz parametr bezpośrednio w swojej metodzie. Pod tym względem nie różni się wcale od żadnego innego parametru.
Ogólnie rzecz biorąc, nie ma absolutnie nic złego w stosowaniu parametrów boolowskich; i oczywiście jakikolwiek parametr determinuje zachowanie lub dlaczego miałbyś je mieć?
źródło
Definiowanie pytania
Twoje tytułowe pytanie brzmi: „Czy [...] jest źle?” - ale co masz na myśli mówiąc „źle”?
Według kompilatora C # lub Java, nie jest źle. Jestem pewien, że jesteś tego świadomy i nie o to pytasz. Obawiam się, że poza tym,
n
programiści mająn+1
różne opinie. Ta odpowiedź przedstawia, co o tym mówi książka Clean Code .Odpowiedź
Clean Code zdecydowanie uzasadnia argumenty funkcji w ogóle:
„Czytnikiem” tutaj może być konsument interfejsu API. Może to być także kolejny program kodujący, który nie wie jeszcze, co robi ten kod - którym możesz być ty za miesiąc. Będą albo przechodzić przez 2 funkcje osobno, albo przez 1 funkcję dwa razy , raz pamiętając
true
i razfalse
na uwadze.Krótko mówiąc, używaj jak najmniej argumentów .
Konkretny przypadek argumentu flagi jest później adresowany bezpośrednio:
Aby odpowiedzieć bezpośrednio na pytania:
zgodnie z Clean Code zaleca się wyeliminowanie tego parametru.
Dodatkowe informacje:
Twój przykład jest raczej prosty, ale nawet tam widzisz prostotę propagującą się do twojego kodu: funkcje bez parametrów wykonują tylko proste przypisania, podczas gdy druga funkcja musi wykonywać arytmetykę logiczną, aby osiągnąć ten sam cel. W tym uproszczonym przykładzie jest to banalna arytmetyka logiczna, ale w prawdziwej sytuacji może być dość złożona.
Widziałem tutaj wiele argumentów, które powinieneś uzależnić od użytkownika interfejsu API, ponieważ robienie tego w wielu miejscach byłoby głupie:
I nie zgadzam się, że coś nie jest optymalne tutaj dzieje. Być może jest to zbyt oczywiste, ale twoje pierwsze zdanie mówi o „znaczeniu unikania [sic] używania parametrów boolowskich do określania zachowania” i to jest podstawa do całego pytania. Nie widzę powodu, aby uczynić to, co jest złe, łatwiejsze dla użytkownika interfejsu API.
Nie wiem, czy wykonujesz testy - w takim przypadku weź także pod uwagę:
źródło