Czy niewłaściwe jest używanie parametru boolowskiego do określania wartości?

39

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ż:

  1. ma więcej kodów niż oryginalna wersja

  2. nie może wyraźnie pokazać, że widoczność warstwy 2 jest przeciwna do opcji podpowiedzi

  3. 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?

ocomfd
źródło
26
Nigdy nie jest źle, jeśli wynikowy kod jest bardziej czytelny i łatwy w utrzymaniu ;-) Poleciłbym użycie jednej metody zamiast dwóch oddzielnych metod.
helb
32
Przedstawiasz przekonujący argument, że pojedyncza implementacja metody ustawiającej te wartości logiczne spowoduje łatwiejsze utrzymanie klasy i zrozumienie jej implementacji. Bardzo dobrze; są to uzasadnione względy. Ale interfejs publiczny klasy nie musi być zdeformowany, aby je dostosować. Jeśli oddzielne metody sprawią, że interfejs publiczny będzie łatwiejszy do zrozumienia i pracy, zdefiniuj swoją setHint(boolean isHintOn)jako metodę prywatną i dodaj public setHintOnoraz setHintOffmetody, które odpowiednio wywołują setHint(true)i setHint(false).
Mark Amery
9
Byłbym bardzo niezadowolony z tych nazw metod: tak naprawdę nie oferują one żadnych korzyści setHint(true|false). Ziemniak Potahto Przynajmniej użyj czegoś takiego jak setHinti unsetHint.
Konrad Rudolph
4
@kevincline Jeśli warunkiem jest jedna nazwa, piszesz isna początku. isValiditd. 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.
Lister

Odpowiedzi:

95

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

if(flag)
    foo.setStateTrue();
else
    foo.setStateFalse();

to powinno być oczywiste, że unikanie parametru jest gorsze niż posiadanie interfejsu API, który umożliwia programowi wywołującemu pisanie

 foo.setState(flag);

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 setHintparametr wymaga mniej kodu podczas implementacji, ale interfejs API w kategoriach setHintOn/ setHintOffwygląda na łatwiejszy w obsłudze dla klienta, można go zaimplementować w ten sposób:

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

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 setStatepowyż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.

Doktor Brown
źródło
DocBrown, czy powiedziałbyś, że właściwą linią podziału jest to, czy ustawienie każdego stanu ma złożone i być może nieodwracalne skutki uboczne? Na przykład, jeśli po prostu przełączasz flagę, która robi to, co mówi na puszce, a nie ma żadnej maszyny stanu, sparametryzujesz różne stany. Podczas gdy na przykład nie sparametryzowałbyś takiej metody 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.
Steve,
15
@ Steve: wciąż próbujesz zaprojektować API na podstawie wymagań, które widzisz po stronie implementacji. Mówiąc wprost: nie ma to żadnego znaczenia. Użyj dowolnego wariantu publicznego interfejsu API, który jest łatwiejszy w użyciu od strony wywołującej. Wewnętrznie zawsze możesz pozwolić, aby dwie metody publiczne nazywały jedną metodę prywatną za pomocą parametru. Lub odwrotnie, możesz pozwolić jednej metodzie z parametrem przełączać się między dwiema prywatnymi metodami o innej logice.
Doc Brown
@ Steve: zobacz moją edycję.
Doc Brown
Biorę wszystkie twoje uwagi - tak naprawdę myślę o tym od strony dzwoniącego (stąd moje odniesienie do „tego, co jest napisane na puszce”) i próbuję sformułować odpowiednią zasadę, w której dzwoniący zwykle spodziewałby się zastosować każde podejście. Wydaje mi się, że reguła polega na tym, czy osoba dzwoniąca oczekuje, że wielokrotne połączenia będą idempotentne, a przejścia stanu będą nieograniczone i nie będą miały złożonych skutków ubocznych. Włączanie i wyłączanie włączania i wyłączania światła w pomieszczeniu byłoby sparametryzowane, a włączanie i wyłączanie regionalnej elektrowni byłoby metodą wielu metod.
Steve
1
@ Steve Więc jeśli użytkownik musi potwierdzić określone ustawienie, 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”, to Toggle()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.
Kamil Drakari
40

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:

Jeśli ciągnąc [sic] Dane z logicznym źródła, takie jak kontrola UI lub źródła danych, wolałbym mieć setSwitch(aValue)niż

if (aValue)
  setOn();
else
  setOff();

Jest to przykład, że interfejs API powinien zostać napisany, aby ułatwić dzwoniącemu, więc jeśli wiemy, skąd pochodzi dzwoniący, powinniśmy zaprojektować interfejs API z uwzględnieniem tych informacji. Argumentuje to również, że czasami możemy zapewnić oba style, jeśli otrzymamy osoby wywołujące na dwa sposoby.

Kolejnym zaleceniem jest użyć flagi wyliczone wartości lub wpisać dać truei falselepiej, nazwy kontekst specyficzne. W przykładzie, showHinta hideHintmoże być lepiej.

Graham Lee
źródło
16
Z mojego doświadczenia wynika, że ​​setSwitch (wartość) prawie zawsze daje mniej ogólny kod niż setOn / setOff właśnie z powodu kodu if / else podanego w odpowiedzi. Mam tendencję do przeklinania programistów, którzy dają mi interfejs API setOn / setOff zamiast setSwitch (wartość).
17 z 26
1
Patrząc na to z podobnego obiektywu: jeśli potrzebujesz na stałe zakodować, jaka będzie wartość, to jedno i drugie. Jeśli jednak musisz ustawić, powiedzmy, dane wejściowe użytkownika, jeśli możesz po prostu przekazać wartość bezpośrednio do, to oszczędza to krok.
Nic Hartley,
@ 17 z 26 jako konkretny kontrprzykład (aby pokazać, że „to zależy”, a nie że jeden jest lepszy od drugiego), w Apple AppKit istnieje -[NSView setNeedsDisplay:]metoda, w której można przekazać, YESczy widok powinien zostać przerysowany, a NOjeśli nie. Prawie nigdy nie musisz tego mówić, więc UIKit po prostu nie -[UIView setNeedsDisplay]ma parametru. Nie ma odpowiedniej -setDoesNotNeedDisplaymetody.
Graham Lee
2
@GrahamLee, myślę, że argument Fowlera jest dość subtelny i naprawdę opiera się na osądzie. Nie użyłbym bool isPremiumflagi 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.
Steve
3

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:

public void setHint(boolean isHintOn)

i:

public void setHintOn()
public void setHintOff()

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:

setHint(true)
// Do your process
…
setHint(false)

podczas gdy druga opcja daje to:

setHintOn()
// Do your process
…
setHintOff()

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 setHintOni setHintOffnasz 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):

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

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:

public void setHint(boolean isHintOn){
    if(isHintOn){
        // Set it On
    } else {
        // Set it Off
    }    
}

Lub nawet:

public void setHint(boolean isHintOn){    
    if(isHintOn){
        setHintOn()
    } else {
        setHintOff()
   }    
}

private void setHintOn(){
   // Set it On
}

private void setHintOff(){
   // Set it Off 
}

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

jesm00
źródło
3
Na marginesie, jeśli jest to coś w rodzaju pseudobloku (jedna instrukcja na początku, jedna na końcu), prawdopodobnie powinieneś użyć begini endlub synonimów, aby jasno powiedzieć, że tak właśnie robią, i sugerować, że początek musi mieć zakończenie i odwrotnie.
Nic Hartley,
Zgadzam się z Nic i dodam: jeśli chcesz mieć pewność, że początki i końce są zawsze połączone, powinieneś również podać idiomy specyficzne dla języka: RAII / scope Guards w C ++, usingbloki w C #, withmenedżery kontekstu instrukcji w Pythonie, przekazywanie ciało jak lambda lub na żądanie obiektu (na przykład składni bloków Ruby), itp
Daniel Pryden
Zgadzam się z obydwoma punktami, próbowałem tylko dać prosty przykład, aby zilustrować inny przypadek (choć jest to zły przykład, jak oboje zauważyliście :)).
jesm00
2

Po 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

bool isBackgroundVisible() {
    return isHintVisible;
}    

bool isContentVisible() {
    return !isHintVisible;
}

(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()i hideHint()- 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.

podwójnie
źródło
Więc tl; dr to: Zalecasz podział na dwie funkcje, ponieważ nie będziesz musiał ich zmieniać podczas dodawania warstw? Jeśli zostanie dodana warstwa4, możemy również powiedzieć „this.layer4.visibility = isHintOn”, więc nie jestem pewien, czy się zgadzam. Jeśli cokolwiek, jest to wadą, ponieważ teraz, gdy dodawana jest warstwa, musimy edytować dwie funkcje, a nie tylko jedną.
Erdrik Ironrose
Nie, ja (słabo) polecam dla jasności ( showHintvs setHintVisibility). Wspomniałem o nowej warstwie tylko dlatego, że OP się o nią martwiła. Ponadto, musimy tylko dodać jedną nową metodę: isLayer4Visible. showHinti hideHintpo prostu ustaw isHintVisibleatrybut na true / false, a to się nie zmienia.
podwójnyYou 10'18
1
@dou You, mówisz, że dłuższy kod nie jest automatycznie mniej konserwowalny. Powiedziałbym, że długość kodu jest jedną z głównych zmiennych w utrzymywalności i przejrzystości, przeważnie złożonością strukturalną. Każdy kod, który staje się dłuższy i bardziej strukturalnie złożony, powinien na niego zasługiwać, w przeciwnym razie proste problemy będą traktowane w kodzie bardziej skomplikowane niż na to zasługują, a baza kodów zyskuje zarośla niepotrzebnych linii (problem „zbyt wielu poziomów pośrednich”).
Steve
@ Steve Całkowicie się zgadzam, że możesz nad-inżynierować rzeczy, tj. Wydłużać kod bez wyjaśniania go - otoh, zawsze możesz skrócić kod kosztem przejrzystości, więc nie ma tutaj relacji 1: 1.
podwójnyYou 10'18
@Steve myśleć „kodu Golf” - Przeprowadzanie takiego kodu i przepisywanie go w kilku liniach ma zazwyczaj uczynić go bardziej zrozumiałym. „Code golf” jest ekstremalny, ale wciąż istnieje wielu programistów, którzy myślą, że łączenie wszystkiego w jeden sprytny wyraz jest „elegancki”, a może nawet szybszy, ponieważ kompilatory nie optymalizują się wystarczająco dobrze.
BlackJack
1

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 metod setStateTrue()i setStateFalse()po prostu niepotrzebnie komplikuje rzeczy bez korzyści.

JacquesB
źródło
1

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:

public enum HintState {
    SHOW_HINT(true, false, true),
    HIDE_HINT(false, true, false);

    private HintState(boolean layer1Visible, boolean layer2Visible, boolean layer3Visible) {
         // constructor body and accessors omitted for clarity
    }
}

A wtedy twój kod dzwoniącego wyglądałby tak:

setHint(HintState.SHOW_HINT);

Twój kod implementacyjny wyglądałby tak:

public void setHint(HintState hint) {
    this.layer1Visible = hint.isLayer1Visible();
    this.layer2Visible = hint.isLayer2Visible();
    this.layer3Visible = hint.isLayer3Visible();
}

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.

Daniel Pryden
źródło
0

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?

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:

function visible() : line 440
function parent() : line 398
function mode() : line 384
function run() : line 5

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

function bar() : line 92
function setVisible() : line 120
function foo() : line 492
function setVisible() : line 120
function run() : line 5

To mylące, jeśli mnie pytasz. Te same setVisiblelinie 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:

  • jasna nazwa funkcji, która sugeruje zamiar bez konieczności znajomości wartości argumentów.
  • funkcja wykonuje jedną akcję
  • nazwa sugeruje mutację lub niezmienne zachowanie
  • rozwiązywać problemy związane z debugowaniem związane ze zdolnością do debugowania języka i narzędzi.

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.

Reactgular
źródło
1
Mylące dla mnie w setVisibleśledzeniu stosu jest to, że wywoływanie setVisible(true)wydaje się wywoływać wywołanie do setVisible(false)(lub odwrotnie, w zależności od tego, w jaki sposób wywołano ten ślad).
David K
0

W prawie każdym przypadku, gdy przekazujesz booleanparametr 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, Enumktóry reprezentuje stan, poprawiłeś zrozumienie swojego kodu.

W tym przykładzie użyto Nodeklasy z JavaFX:

public enum Visiblity
{
    SHOW, HIDE

    public boolean toggleVisibility(@Nonnull final Node node) {
        node.setVisible(!node.isVisible());
    }
}

jest zawsze lepszy niż, jak na wielu JavaFXobiektach:

public void setVisiblity(final boolean flag);

ale myślę .setVisible()i .setHidden()są najlepszym rozwiązaniem dla sytuacji, w której flaga jest, booleanponieważ 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. EnumSetistnieje właśnie z tego powodu.

Ed Mann ma naprawdę dobry post na blogu na ten temat. Właśnie miałem sparafrazować to, co mówi, więc aby nie powielać wysiłku, po prostu opublikuję link do jego postu na blogu jako uzupełnienie tej odpowiedzi.


źródło
0

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

Erik Eidt
źródło
Co się stanie, jeśli projektujesz zintegrowany system i ostatecznie jesteś zarówno producentem kodu, jak i „konsumentem klienta” kodu? W jaki sposób osoba stojąca w butach konsumenta konsumenckiego powinna sformułować swoją preferencję dla jednego podejścia względem drugiego?
Steve
@ Steve, Jako klient konsumujący, wiesz, czy przekazujesz stałą, czy zmienną. Jeśli przekazujesz stałe, preferuj przeciążenia bez parametru.
Erik Eidt,
Ale chcę wyjaśnić, dlaczego tak powinno być. Dlaczego nie zastosować wyliczeń dla ograniczonej liczby stałych, ponieważ jest to lekka składnia w większości języków zaprojektowana właśnie do tego celu?
Steve
@ Steve, jeśli wiemy w czasie projektowania / kompilacji, że klienci używają stałej wartości (prawda / fałsz) we wszystkich przypadkach, oznacza to, że tak naprawdę istnieją dwie różne konkretne metody zamiast jednej metody uogólnionej (która przyjmuje parametr). Argumentowałbym przeciwko wprowadzeniu ogólności sparametryzowanej metody, gdy nie jest ona używana - jest to argument YAGNI.
Erik Eidt,
0

Istnieją dwa czynniki do zaprojektowania:

  • API: jaki interfejs przedstawiasz użytkownikowi,
  • Wdrożenie: jasność, łatwość konserwacji itp.

Nie należy ich łączyć.

Zupełnie dobrze jest:

  • mieć wiele metod w interfejsie API delegowanych do pojedynczej implementacji,
  • mieć jedną metodę wysyłania API do wielu implementacji w zależności od warunku.

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/ switchwartoś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):

this.layer1.visible = isHintOn;
this.layer2.visible = ! isHintOn;
this.layer3.visible = isHintOn;

Dlaczego layer2przeł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:

for (auto layer : this.mainLayers) { // layer2
    layer.visible = ! isHintOn;
}
for (auto layer : this.hintLayers) { // layer1 and layer3
    layer.visible = isHintOn;
}

Kod mówi sam za siebie, jasne jest, dlaczego istnieją dwie grupy, a ich zachowanie się różni.

Matthieu M.
źródło
0

Oddzielnie od pytania setOn() + setOff()vs set(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 .:

setHint(false)

vs

setHint(Visibility::HIDE)

Dzięki wyliczeniu będzie znacznie łatwiej rozszerzyć, gdy ktoś zdecyduje, że chce opcji „w razie potrzeby”:

enum class Visibility {
  SHOW,
  HIDE,
  IF_NEEDED // New
}

vs

setHint(false)
setHint(true)
setHintAutomaticMode(true) // New
Phil
źródło
0

Według ... wiem, jak ważne jest unikanie używania parametrów boolowskich do określania zachowania

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ć?

AnoE
źródło
0

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, nprogramiści mają n+1różne opinie. Ta odpowiedź przedstawia, co o tym mówi książka Clean Code .

Odpowiedź

Clean Code zdecydowanie uzasadnia argumenty funkcji w ogóle:

Argumenty są trudne. Biorą dużo mocy pojęciowej. [...] nasi czytelnicy musieliby ją interpretować za każdym razem, gdy ją widzieli.

„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 truei raz falsena uwadze.
Krótko mówiąc, używaj jak najmniej argumentów .

Konkretny przypadek argumentu flagi jest później adresowany bezpośrednio:

Argumenty dotyczące flagi są brzydkie. Przekazanie funkcji logicznej na funkcję jest naprawdę okropną praktyką. Natychmiast komplikuje to sygnaturę metody, głośno głosząc, że ta funkcja wykonuje więcej niż jedną rzecz. Robi jedną rzecz, jeśli flaga jest prawdziwa, a drugą, jeśli flaga jest fałszywa!

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:

if (isAfterSunset) light.TurnOn();
else light.TurnOff();

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ę:

Argumenty są jeszcze trudniejsze z testowego punktu widzenia. Wyobraź sobie trudność napisania wszystkich przypadków testowych, aby upewnić się, że wszystkie kombinacje argumentów działają poprawnie. Jeśli nie ma argumentów, jest to trywialne.

R. Schmitz
źródło
Naprawdę zakopałeś tutaj lede: „... w pierwszym zdaniu wspominasz o„ znaczeniu unikania [sic] używania parametrów boolowskich do określania zachowania ”i to jest podstawa do całego pytania. Nie rozumiem powód, aby uczynić to, co nie jest łatwe do zrobienia dla użytkownika interfejsu API ”. Jest to interesujący punkt, ale raczej podważasz swój własny argument w ostatnim akapicie.
Wildcard
Pochowałeś lede? Trzon tej odpowiedzi brzmi: „używaj jak najmniejszej liczby argumentów”, co wyjaśniono w pierwszej połowie tej odpowiedzi. Wszystko po tym to tylko dodatkowe informacje: obalenie sprzecznego argumentu (przez innego użytkownika, nie OP) i coś, co nie dotyczy wszystkich.
R. Schmitz
W ostatnim akapicie próbuję wyjaśnić, że pytanie tytułowe nie jest wystarczająco dobrze zdefiniowane, aby można było na nie odpowiedzieć. OP pyta, czy to „źle”, ale nie mówi, według kogo lub co. Według kompilatora? Wygląda na poprawny kod, więc nie jest źle. Zgodnie z książką Clean Code? Używa argumentu flagi, więc tak, to „źle”. Zamiast tego piszę „zalecane”, ponieważ pragmatyczne> dogmatyczne. Czy uważasz, że muszę to wyjaśnić?
R. Schmitz
więc poczekaj, twoja obrona swojej odpowiedzi jest taka, że ​​pytanie tytułowe jest zbyt niejasne, aby można było na nie odpowiedzieć? : D Okay ... Właściwie pomyślałem, że cytowany przeze mnie punkt był interesujący.
Wildcard
1
Teraz wyraźnie jasne; dobra robota!
Wildcard