Laut Ist es falsch , einen Booleschen Parameter zu verwenden Verhalten zu bestimmen? Ich weiß, wie wichtig es ist, keine booleschen Parameter zu verwenden, um ein Verhalten zu bestimmen, z.
Originalfassung
public void setState(boolean flag){
if(flag){
a();
}else{
b();
}
c();
}
neue Version:
public void setStateTrue(){
a();
c();
}
public void setStateFalse(){
b();
c();
}
Aber wie wäre es mit dem Fall, dass der boolesche Parameter verwendet wird, um Werte anstelle von Verhaltensweisen zu bestimmen? z.B:
public void setHint(boolean isHintOn){
this.layer1.visible=isHintOn;
this.layer2.visible=!isHintOn;
this.layer3.visible=isHintOn;
}
Ich versuche, das isHintOn-Flag zu entfernen und zwei separate Funktionen zu erstellen:
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;
}
Die geänderte Version scheint jedoch weniger wartbar zu sein, weil:
Es hat mehr Codes als die Originalversion
Es kann nicht eindeutig gezeigt werden, dass die Sichtbarkeit von Layer2 der Hinweisoption entgegengesetzt ist
Wenn eine neue Ebene (zB: Ebene4) hinzugefügt wird, muss ich hinzufügen
this.layer4.visible=false;
und
this.layer4.visible=true;
in setHintOn () und setHintOff () getrennt
Meine Frage ist also, ob es immer noch empfehlenswert ist, diesen booleschen Parameter zu entfernen, wenn der boolesche Parameter nur zur Bestimmung von Werten verwendet wird, aber nicht für Verhaltensweisen (z. B. kein if-else für diesen Parameter).
setHint(boolean isHintOn)
als private Methode und fügen Sie publicsetHintOn
undsetHintOff
Methoden hinzu, die bzw.setHint(true)
und aufrufensetHint(false)
.setHint(true|false)
. Kartoffel-Potahto. Zumindest etwas wiesetHint
und verwendenunsetHint
.is
am Anfang.isValid
usw. Warum sollte man das für zwei Wörter ändern? Dabei liegt "natürlicher" im Auge des Betrachters. Wenn Sie es als englischen Satz aussprechen möchten, dann wäre es für mich natürlicher, "wenn der Hinweis auf" mit einem "eingeblendeten" steht.Antworten:
Das API-Design sollte sich auf das konzentrieren, was für einen Client der API von der aufrufenden Seite am nützlichsten ist .
Wenn diese neue API beispielsweise erfordert, dass der Aufrufer regelmäßig Code wie diesen schreibt
Dann sollte klar sein, dass das Vermeiden des Parameters schlimmer ist als eine API, die es dem Aufrufer ermöglicht, zu schreiben
Die frühere Version erzeugt nur ein Problem, das dann auf der aufrufenden Seite (und wahrscheinlich mehr als einmal) gelöst werden muss. Das erhöht weder die Lesbarkeit noch die Wartbarkeit.
Die Implementierungsseite sollte jedoch nicht bestimmen, wie die öffentliche API aussieht. Wenn eine Funktion wie
setHint
mit einem Parameter bei der Implementierung weniger Code benötigt, eine API für einen Client jedoch einfacher zu verwenden istsetHintOn
/setHintOff
aussieht, kann sie folgendermaßen implementiert werden:Obwohl die öffentliche API keinen booleschen Parameter hat, gibt es hier keine doppelte Logik, sodass nur eine Stelle geändert werden muss, wenn eine neue Anforderung (wie im Beispiel der Frage) eintrifft.
Dies funktioniert auch umgekehrt: Wenn die
setState
obige Methode zwischen zwei verschiedenen Codeteilen wechseln muss, können diese Codeteile in zwei verschiedene private Methoden umgestaltet werden. Daher ist es meiner Meinung nach nicht sinnvoll, anhand der Interna nach einem Kriterium für die Entscheidung zwischen "einem Parameter / einer Methode" und "null Parametern / zwei Methoden" zu suchen. Sehen Sie sich jedoch an, wie Sie die API in der Rolle eines Konsumenten davon sehen möchten.Versuchen Sie im Zweifelsfall die Verwendung von "Test Driven Development" (TDD), um über die öffentliche API und deren Verwendung nachzudenken.
quelle
SetLoanFacility(bool enabled)
, weil ein Darlehen bereitgestellt wurde, ist es möglicherweise nicht einfach, es wieder zu entfernen, und die beiden Optionen beinhalten möglicherweise eine völlig andere Logik - und Sie möchten das Erstellen trennen / Methoden entfernen.Toggle()
ist nicht die richtige Funktion bereitzustellen. Das ist der ganze Punkt; Wenn es dem Anrufer nur darum geht, "es zu ändern" und nicht "wie es endet",Toggle()
ist die Option, die zusätzliche Überprüfungen und Entscheidungen vermeidet. Ich würde das nicht als GEMEINSAM bezeichnen, noch würde ich empfehlen, es ohne guten Grund zur Verfügung zu stellen, aber wenn der Benutzer einen Umschalter benötigt, würde ich ihm einen Umschalter geben.Martin Fowler zitiert Kent Beck , indem er separate
setOn()
setOff()
Methoden empfiehlt , sagt aber auch, dass dies nicht als unverletzlich angesehen werden sollte:Eine weitere Empfehlung ist ein Aufzählungswert oder Flags zu verwenden geben zu geben
true
undfalse
besser, kontextspezifische Namen. In deinem BeispielshowHint
undhideHint
könnte besser sein.quelle
-[NSView setNeedsDisplay:]
Methode, mit der Sie übergeben,YES
ob eine Ansicht neu gezeichnet werden soll undNO
wenn nicht. Sie müssen es so gut wie nie ablehnen, daher hat UIKit einfach-[UIView setNeedsDisplay]
keinen Parameter. Es hat nicht die entsprechende-setDoesNotNeedDisplay
Methode.bool isPremium
In seinem Beispiel würde ich kein Flag verwenden, aber ich würde ein enum (BookingType bookingType
) verwenden, um dieselbe Methode zu parametrisieren, es sei denn, die Logik für jede Buchung war ganz anders. Die "verworrene Logik", auf die sich Fowler bezieht, ist oft wünschenswert, wenn man in der Lage sein will, den Unterschied zwischen den beiden Modi zu erkennen. Und wenn sie sich radikal unterscheiden, würde ich die parametrisierte Methode extern verfügbar machen und die separaten Methoden intern implementieren.Ich denke, Sie mischen zwei Dinge in Ihrem Beitrag, die API und die Implementierung. In beiden Fällen gibt es meines Erachtens keine strenge Regel, die Sie immer anwenden können, aber Sie sollten diese beiden Punkte unabhängig voneinander berücksichtigen (so weit wie möglich).
Beginnen wir mit der API, beides:
und:
sind gültige Alternativen, abhängig davon, was Ihr Objekt bieten soll und wie Ihre Kunden die API verwenden werden. Wie Doc betonte, ist die erste Option sinnvoller, wenn Ihre Benutzer bereits über eine boolesche Variable verfügen (über ein UI-Steuerelement, eine Benutzeraktion, eine externe API usw.), andernfalls erzwingen Sie lediglich eine zusätzliche if-Anweisung im Code des Clients . Wenn Sie zum Beispiel den Hinweis zu Beginn eines Prozesses auf true und am Ende auf false setzen, erhalten Sie mit der ersten Option Folgendes:
Die zweite Option bietet Ihnen Folgendes:
Welche IMO ist viel lesbarer, so werde ich mit der zweiten Option in diesem Fall gehen. Offensichtlich hindert Sie nichts daran, beide Optionen anzubieten (oder mehr, Sie könnten eine Aufzählung verwenden, wie Graham sagte, wenn das zum Beispiel sinnvoller ist).
Der Punkt ist, dass Sie Ihre API basierend darauf auswählen sollten, was das Objekt tun soll und wie die Clients es verwenden werden, nicht basierend darauf, wie Sie es implementieren werden.
Dann müssen Sie auswählen, wie Sie Ihre öffentliche API implementieren. Nehmen wir an, wir haben die Methoden
setHintOn
undsetHintOff
als öffentliche API ausgewählt und sie teilen diese gemeinsame Logik wie in Ihrem Beispiel. Sie können diese Logik leicht mit einer privaten Methode (aus Doc kopierter Code) abstrahieren:Nehmen wir umgekehrt an, wir haben
setHint(boolean isHintOn)
eine unserer APIs ausgewählt, aber wir kehren Ihr Beispiel um, und zwar aus irgendeinem Grund, weil das Setzen des Hinweises auf "Ein" völlig anders ist als das Setzen auf "Aus". In diesem Fall könnten wir es wie folgt implementieren:Oder auch:
Der Punkt ist, dass wir in beiden Fällen zuerst unsere öffentliche API ausgewählt und dann unsere Implementierung angepasst haben, um sie an die gewählte API (und die Einschränkungen, die wir haben) anzupassen, und nicht umgekehrt.
Übrigens, ich denke, dasselbe gilt für den Beitrag, den Sie über die Verwendung eines booleschen Parameters zur Bestimmung des Verhaltens verlinkt haben. Sie sollten sich also eher für Ihren speziellen Anwendungsfall als für eine harte Regel entscheiden (obwohl in diesem speziellen Fall normalerweise das Richtige zu tun ist ist es in mehrere Funktionen zu brechen).
quelle
begin
und /end
oder Synonyme verwenden, nur um deutlich zu machen, dass dies das ist, was sie tun, und um zu implizieren, dass a Anfang muss ein Ende haben und umgekehrt.using
Blöcke in C #,with
Anweisungskontext-Manager in Python, Übergabe der Körper als Lambda oder aufrufbares Objekt (zB Ruby-Block-Syntax) usw.Das Wichtigste zuerst: Code ist nicht automatisch weniger wartbar, nur weil er etwas länger ist. Klarheit ist wichtig.
Wenn Sie sich wirklich nur mit Daten beschäftigen, ist das, was Sie haben, ein Setter für eine boolesche Eigenschaft. In diesem Fall möchten Sie möglicherweise nur diesen Wert direkt speichern und die Sichtbarkeit des Layers ableiten, d. H
(Ich habe mir die Freiheit genommen, den Ebenen tatsächliche Namen zu geben. Wenn Sie dies nicht in Ihrem ursprünglichen Code haben, würde ich damit beginnen.)
Sie haben immer noch die Frage, ob Sie eine
setHintVisibility(bool)
Methode haben sollen. Persönlich würde ich empfehlen, es durch eineshowHint()
undhideHint()
-Methode zu ersetzen - beide sind sehr einfach und müssen beim Hinzufügen von Ebenen nicht geändert werden. Es ist jedoch kein eindeutiges Richtig / Falsch.Wenn nun der Aufruf der Funktion tatsächlich die Sichtbarkeit dieser Ebenen ändern sollte, haben Sie tatsächlich Verhalten. In diesem Fall würde ich auf jeden Fall separate Methoden empfehlen.
quelle
showHint
vssetHintVisibility
). Ich erwähnte die neue Schicht nur, weil OP sich darüber Sorgen machte. Auch wir brauchen nur eine neue Methode hinzuzufügen:isLayer4Visible
.showHint
undhideHint
setzen Sie dasisHintVisible
Attribut einfach auf true / false und das ändert sich nicht.Boolesche Parameter sind im zweiten Beispiel in Ordnung. Wie Sie bereits herausgefunden haben, sind boolesche Parameter an sich nicht problematisch. Es ist ein Schaltverhalten basierend auf einem Flag, was problematisch ist.
Das erste Beispiel ist jedoch problematisch, da die Benennung auf eine Setter-Methode hinweist, die Implementierungen jedoch etwas anderes zu sein scheinen. Sie haben also das Antimuster für Verhaltensänderungen und eine irreführend benannte Methode. Aber wenn die Methode tatsächlich ein regulärer Setter ist (ohne Verhaltensänderung), gibt es kein Problem mit
setState(boolean)
. Zwei Methoden zu habensetStateTrue()
undsetStateFalse()
die Dinge unnötig zu komplizieren, bringt keinen Nutzen.quelle
Eine andere Möglichkeit, dieses Problem zu lösen, besteht darin, ein Objekt einzuführen, das jeden Hinweis darstellt, und das Objekt für die Bestimmung der diesem Hinweis zugeordneten Booleschen Werte verantwortlich zu machen. Auf diese Weise können Sie neue Permutationen hinzufügen, anstatt einfach zwei Boolesche Zustände zu haben.
In Java können Sie beispielsweise Folgendes tun:
Und dann würde Ihr Anrufercode so aussehen:
Und Ihr Implementierungscode würde so aussehen:
Dies hält den Implementierungscode und den Aufrufercode kurz, im Gegenzug zum Definieren eines neuen Datentyps, der stark typisierte, benannte Absichten eindeutig den entsprechenden Mengen von Zuständen zuordnet. Ich denke, das ist überall besser.
quelle
Wenn ich Zweifel an solchen Dingen habe. Ich stelle mir gerne vor, wie der Stack-Trace aussehen würde.
Ich habe viele Jahre an einem PHP-Projekt gearbeitet, das die gleiche Funktion wie ein Setter und Getter verwendet hat . Wenn Sie null übergeben , wird der Wert zurückgegeben, andernfalls wird er festgelegt. Es war schrecklich, damit zu arbeiten .
Hier ist ein Beispiel für einen Stack-Trace:
Sie hatten keine Ahnung vom internen Status und das Debuggen wurde schwieriger. Es gibt eine Reihe anderer negativer Nebenwirkungen, aber versuchen Sie herauszufinden, ob ein ausführlicher Funktionsname und die Klarheit von Funktionen von Nutzen sind, wenn Funktionen eine einzelne Aktion ausführen .
Stellen Sie sich nun vor, wie Sie mit dem Stack-Trace für eine Funktion arbeiten, deren A- oder B-Verhalten auf einem booleschen Wert basiert.
Das ist verwirrend, wenn Sie mich fragen. Dieselben
setVisible
Linien ergeben zwei unterschiedliche Tracepfade.Also zurück zu deiner Frage. Stellen Sie sich vor, wie der Stack-Trace aussehen wird, wie er einer Person mitteilt, was gerade passiert, und fragen Sie sich, ob Sie einer zukünftigen Person beim Debuggen des Codes helfen.
Hier sind einige Tipps:
Manchmal sieht Code unter dem Mikroskop übermäßig aus, aber wenn Sie sich auf das Gesamtbild beschränken, lässt ihn ein minimalistischer Ansatz verschwinden. Wenn Sie es brauchen, um aufzufallen, damit Sie es warten können. Das Hinzufügen vieler kleiner Funktionen wirkt möglicherweise zu ausführlich, verbessert jedoch die Wartbarkeit, wenn sie in einem größeren Kontext verwendet werden.
quelle
setVisible
Stapelablaufverfolgung verwirrt, ist, dass ein AufrufsetVisible(true)
anscheinend zu einem Aufruf führtsetVisible(false)
(oder umgekehrt, je nachdem, wie Sie diese Ablaufverfolgung erhalten haben).In fast allen Fällen, in denen Sie einen
boolean
Parameter als Flag an eine Methode übergeben , um das Verhalten von etwas zu ändern, sollten Sie eine explizite und typsichere Methode in Betracht ziehen, um dies zu tun.Wenn Sie nichts weiter tun, als eine zu verwenden
Enum
, die den Status darstellt, haben Sie das Verständnis Ihres Codes verbessert.In diesem Beispiel wird die
Node
Klasse von verwendetJavaFX
:ist immer besser als, wie man es bei vielen
JavaFX
Objekten findet:Aber ich denke
.setVisible()
und bin.setHidden()
die beste Lösung für die Situation, in der die Flagge eine ist,boolean
da sie die expliziteste und am wenigsten ausführliche ist.Bei etwas mit Mehrfachauswahl ist dies sogar noch wichtiger.
EnumSet
existiert nur aus diesem Grund.quelle
Wenn Sie sich für einen Ansatz für eine Schnittstelle entscheiden, die einen (booleschen) Parameter übergibt, oder für überladene Methoden ohne diesen Parameter, achten Sie auf die konsumierenden Clients.
Wenn alle Verwendungen konstante Werte (z. B. true, false) übergeben würden, spricht dies für die Überladung.
Wenn alle Verwendungen einen variablen Wert übergeben würden, spricht dies für die Methode mit Parameteransatz.
Wenn keines dieser Extreme zutrifft, bedeutet dies, dass es eine Mischung aus Client-Verwendungen gibt. Sie müssen also entscheiden, ob beide Formulare unterstützt werden sollen oder ob eine Kategorie von Clients an den anderen (was für sie unnatürlicher ist) Stil angepasst werden soll.
quelle
Es gibt zwei Überlegungen zum Entwerfen:
Sie sollten nicht zusammengeführt werden.
Es ist vollkommen in Ordnung:
Daher ist jedes Argument, das versucht, die Kosten / Nutzen eines API-Designs mit den Kosten / Nutzen eines Implementierungsdesigns in Einklang zu bringen, zweifelhaft und sollte sorgfältig geprüft werden.
Auf der API-Seite
Als Programmierer bevorzuge ich generell programmierbare APIs. Code ist viel klarer, wenn ich einen Wert weiterleiten kann, als wenn ich eine
if
/switch
-Anweisung für den Wert benötige , um zu entscheiden, welche Funktion aufgerufen werden soll.Letzteres kann erforderlich sein, wenn jede Funktion andere Argumente erwartet.
In Ihrem Fall
setState(type value)
scheint also eine einzige Methode besser zu sein.Allerdings gibt es nichts Schlimmeres , als namenlos ist
true
,false
,2
usw ... die magischen Werte haben keine Bedeutung auf eigene Faust. Vermeiden Sie primitive Besessenheit und nehmen Sie starkes Tippen an.So wird aus einem API - POV, ich will:
setState(State state)
.Auf der Implementierungsseite
Ich empfehle, alles zu tun, was einfacher ist.
Wenn die Methode einfach ist, ist es am besten zusammengehalten. Wenn der Kontrollfluss verschlungen ist, ist es am besten, ihn in mehrere Methoden zu unterteilen, die sich jeweils mit einem Teilfall oder einem Schritt der Pipeline befassen.
Betrachten Sie schließlich die Gruppierung .
In Ihrem Beispiel (mit zusätzlichen Leerzeichen für die Lesbarkeit):
Warum
layer2
widersetzt sich der Trend? Ist es ein Feature oder ein Bug?Könnte es möglich sein, 2 Listen zu haben
[layer1, layer3]
und[layer2]
mit einem expliziten Namen den Grund für die Gruppierung anzugeben und dann über diese Listen zu iterieren.Zum Beispiel:
Der Code spricht für sich, es ist klar, warum es zwei Gruppen gibt und sich ihr Verhalten unterscheidet.
quelle
Abgesehen von der
setOn() + setOff()
vs-set(flag)
Frage würde ich sorgfältig prüfen, ob ein Boolescher Typ hier am besten ist. Sind Sie sicher, dass es niemals eine dritte Option geben wird?Es könnte sich lohnen, eine Aufzählung anstelle eines Booleschen zu betrachten. Dies ermöglicht nicht nur Erweiterbarkeit, sondern macht es auch schwieriger, den Booleschen Wert in die falsche Richtung zu lenken, z.
vs
Mit der Aufzählung wird es viel einfacher, die Liste zu erweitern, wenn jemand entscheidet, dass er eine Option "bei Bedarf" wünscht:
vs
quelle
Ich würde vorschlagen, dieses Wissen neu zu bewerten.
Zunächst sehe ich nicht die Schlussfolgerung, die Sie in der von Ihnen verknüpften SE-Frage vorschlagen. Meist geht es darum, einen Parameter über mehrere Schritte von Methodenaufrufen weiterzuleiten, wobei er sehr weit in der Kette ausgewertet wird.
In Ihrem Beispiel bewerten Sie den Parameter direkt in Ihrer Methode. Insofern unterscheidet es sich überhaupt nicht von anderen Parametern.
Im Allgemeinen ist die Verwendung von Booleschen Parametern absolut nichts Falsches. und offensichtlich wird jeder Parameter ein Verhalten bestimmen, oder warum sollten Sie es überhaupt haben?
quelle
Die Frage definieren
Ihre Titelfrage lautet "Ist es falsch, [...]?" - aber was meinst du mit "falsch"?
Laut einem C # - oder Java-Compiler ist das nicht falsch. Ich bin mir sicher, dass Sie sich dessen bewusst sind, und es ist nicht das, wonach Sie fragen. Abgesehen davon haben wir leider nur unterschiedliche Meinungen der
n
Programmierern+1
. Diese Antwort zeigt, was das Buch Clean Code dazu zu sagen hat.Antworten
Clean Code spricht stark gegen Funktionsargumente im Allgemeinen:
Ein "Leser" kann hier der API-Verbraucher sein. Es kann auch der nächste Codierer sein, der noch nicht weiß, was dieser Code tut - und der Sie in einem Monat sein könnte. Sie durchlaufen entweder 2 Funktionen getrennt oder 1 Funktion zweimal ,
true
einmalfalse
im Gedächtnis und einmal .Kurz gesagt, verwenden Sie so wenig Argumente wie möglich .
Der spezielle Fall eines Flag-Arguments wird später direkt angesprochen:
Um Ihre Fragen direkt zu beantworten:
Laut Clean Code wird empfohlen, diesen Parameter zu entfernen.
Zusätzliche Information:
Ihr Beispiel ist recht einfach, aber auch dort können Sie die Einfachheit erkennen, die sich in Ihrem Code ausbreitet: Die Funktionen ohne Parameter führen nur einfache Zuweisungen aus, während die andere Funktion eine Boolesche Arithmetik ausführen muss, um dasselbe Ziel zu erreichen. In diesem vereinfachten Beispiel ist es eine triviale Boolesche Arithmetik, die in einer realen Situation jedoch recht komplex sein kann.
Ich habe hier viele Argumente gesehen, die Sie vom API-Benutzer abhängig machen sollten, weil es dumm wäre, dies an vielen Stellen tun zu müssen:
Ich kann bestätigen , dass etwas nicht optimal hier geschieht. Vielleicht ist es zu offensichtlich, um es zu sehen, aber Ihr allererster Satz erwähnt "die Wichtigkeit, keine booleschen Parameter zu verwenden, um ein Verhalten zu bestimmen", und das ist die Basis für die ganze Frage. Ich sehe keinen Grund, dem API-Benutzer das zu vereinfachen, was schlecht ist.
Ich weiß nicht, ob Sie testen - in diesem Fall sollten Sie auch Folgendes berücksichtigen:
quelle