Lieber 2 Methoden mit klarer Bedeutung oder nur 1 Dual-Use-Methode?

30

Ist es zur Vereinfachung der Benutzeroberfläche besser, die getBalance()Methode nicht zu haben ? Übergeben 0an charge(float c);wird das gleiche Ergebnis geben:

public class Client {
    private float bal;
    float getBalance() { return bal; }
    float charge(float c) {
        bal -= c;
        return bal;
    }
}

Vielleicht eine Notiz machen javadoc? Oder überlassen Sie es einfach dem Klassenbenutzer, herauszufinden, wie er das Gleichgewicht herstellen kann?

David
quelle
26
Ich gehe großzügigerweise davon aus, dass dies beispielhafter Code ist und dass Sie keine Gleitkomma-Mathematik zum Speichern von Geldwerten verwenden. Sonst müsste ich alles predigen. :-)
corsiKa
1
@corsiKa nah. Das ist nur ein Beispiel. Aber ja, ich hätte Float verwendet, um Geld in einer echten Klasse darzustellen ... Vielen Dank, dass Sie mich an die Gefahren von Gleitkommazahlen erinnert haben.
David
10
Einige Sprachen unterscheiden effektiv zwischen Mutatoren und Accessoren. Es wäre furchtbar ärgerlich, nicht in der Lage zu sein, das Gleichgewicht einer Instanz zu erhalten, die nur als Konstante zugänglich ist!
JDługosz

Antworten:

90

Sie scheinen anzunehmen, dass die Komplexität einer Schnittstelle an der Anzahl der Elemente gemessen wird (in diesem Fall an den Methoden). Viele würden argumentieren, dass man sich daran erinnern muss, dass die chargeMethode verwendet werden kann, um den Rest von a zurückzugeben, was Clientviel komplexer ist als das zusätzliche Element der getBalanceMethode. Es ist viel einfacher, die Dinge expliziter zu machen, insbesondere bis zu dem Punkt, an dem es keine Mehrdeutigkeit mehr gibt, unabhängig von der höheren Anzahl von Elementen in der Schnittstelle.

Außerdem anrufen charge(0) verstößt das gegen das Prinzip des geringsten Erstaunens , das auch als WTFs pro Minute- Metrik (von Clean Code, Bild unten) bezeichnet wird, und erschwert es neuen Mitgliedern des Teams (oder jetzigen, nach einer Weile vom Code entfernt), bis Sie verstehen, dass der Anruf tatsächlich verwendet wird, um das Guthaben zu erhalten. Überlegen Sie, wie andere Leser reagieren würden:

Bildbeschreibung hier eingeben

Außerdem verstößt die Signatur der chargeMethode gegen die Richtlinien , eine und nur eine Sache und eine Trennung zwischen Befehlen und Abfragen durchzuführen , da das Objekt seinen Status ändert und gleichzeitig einen neuen Wert zurückgibt.

Alles in allem glaube ich, dass die einfachste Schnittstelle in diesem Fall wäre:

public class Client {
  private float bal;
  float getBalance() { return bal; }
  void charge(float c) { bal -= c; }
}
MichelHenrich
quelle
25
Der Punkt zur Trennung von Befehlen und Abfragen ist äußerst wichtig, IMO. Stellen Sie sich vor, Sie sind ein neuer Entwickler in diesem Projekt. Wenn Sie den Code durchsehen, ist sofort klar, dass er getBalance()einen bestimmten Wert zurückgibt und nichts ändert. Auf der anderen Seite charge(0)sieht es so aus, als würde es wahrscheinlich etwas ändern ... Vielleicht wird ein PropertyChanged-Ereignis gesendet? Und was gibt es zurück, den vorherigen Wert oder den neuen Wert? Jetzt müssen Sie es in den Dokumenten nachschlagen und Ihren Verstand verschwenden, was mit einer klareren Methode hätte vermieden werden können.
Magier Xy
19
Die Verwendung charge(0)als Methode zum Abrufen des Gleichgewichts, da es zufällig keine Auswirkungen hat, bringt Sie jetzt mit diesem Implementierungsdetail in Kontakt. Ich kann mir leicht vorstellen, dass eine zukünftige Anforderung, bei der die Verfolgung der Anzahl der Ladungen relevant wird, zu einem Schmerzpunkt wird, wenn Ihre Codebasis mit Ladungen übersät ist, die eigentlich keine Ladungen sind. Sie sollten Schnittstellenelemente bereitstellen, die das bedeuten, was Ihre Kunden tun müssen, und keine Elemente, die nur das tun, was Ihre Kunden tun müssen.
Ben
12
Die CQS-Ermahnung ist nicht unbedingt angemessen. charge()Wenn diese Methode in einem gleichzeitigen System atomar ist, kann es für eine Methode angebracht sein, den neuen oder alten Wert zurückzugeben.
Dietrich Epp
3
Ich fand Ihre beiden Zitate für CQRS äußerst nicht überzeugend. Ich mag zum Beispiel generell Meyers Ideen, aber es ist willkürlich und unrobust, wenn ich mir den Rückgabetyp einer Funktion ansehe, um festzustellen, ob sie etwas mutiert. Viele gleichzeitige oder unveränderliche Datenstrukturen erfordern Mutation + Rückgaben. Wie würden Sie an einen String anhängen, ohne CQRS zu verletzen? Hast du bessere Zitate?
user949300
6
Fast kein Code entspricht der Befehlsabfragetrennung. Es ist in keiner der gängigen objektorientierten Sprachen idiomatisch und wird von den integrierten APIs aller Sprachen, die ich jemals verwendet habe, verletzt. Es scheint also bizarr, es als "best practice" zu bezeichnen; Können Sie auch nur ein Softwareprojekt nennen, das tatsächlich diesem Muster folgt?
Mark Amery
28

IMO, das Ersetzen getBalance()durch charge(0)in Ihrer gesamten Anwendung ist keine Vereinfachung. Ja, es sind weniger Zeilen, aber es verschleiert die Bedeutung der charge()Methode, was möglicherweise zu Kopfschmerzen führen kann, wenn Sie oder eine andere Person diesen Code erneut aufrufen müssen.

Obwohl sie möglicherweise das gleiche Ergebnis erzielen, entspricht das Abrufen des Kontostands nicht einer Belastung von Null. Daher ist es wahrscheinlich am besten, Ihre Bedenken zu trennen. Wenn Sie beispielsweise bei jeder Kontotransaktion Änderungen charge()am Protokoll vornehmen mussten, besteht jetzt ein Problem, und die Funktionalität muss ohnehin getrennt werden.

Matt Dalzell
quelle
13

Denken Sie daran, dass sich Ihr Code selbst dokumentieren sollte. Wenn ich anrufe charge(x), erwarte ich eine xGebühr. Informationen zum Gleichgewicht sind zweitrangig. Außerdem weiß ich möglicherweise nicht, wie charge()es implementiert wird, wenn ich es aufrufe, und ich weiß definitiv nicht, wie es morgen implementiert wird. Betrachten Sie dieses potenzielle zukünftige Update beispielsweise als charge():

float charge(float c) {
    lockDownUserAccountUntilChargeClears();
    bal -= c;
    Unlock();
    return bal;
}

Plötzlich charge()sieht es nicht mehr so ​​gut aus, wenn man verwendet , um das Gleichgewicht herzustellen.

David sagt Reinstate Monica
quelle
Ja! Ein guter Punkt, der chargeviel schwerer ist.
Deduplizierer
2

Es charge(0);ist eine schlechte Idee, den Kontostand abzurufen: Eines Tages könnte jemand dort einen Code hinzufügen, um die getätigten Gebühren zu protokollieren, ohne die andere Verwendung der Funktion zu bemerken. Jedes Mal, wenn jemand den Kontostand abruft, wird dieser Code als Gebühr protokolliert. (Es gibt Möglichkeiten, dies zu umgehen, beispielsweise eine bedingte Anweisung, die Folgendes sagt:

if (c > 0) {
    // make and log charge
}
return bal;

Diese setzen jedoch voraus, dass der Programmierer sie implementieren kann. Dies wird er nicht tun, wenn nicht sofort klar ist, dass sie erforderlich sind.

Kurz gesagt: Verlassen Sie sich nicht darauf, dass Ihre Benutzer oder die Nachfolger Ihres Programmierers erkennen, dass dies charge(0);der richtige Weg ist, um das Gleichgewicht zu finden, denn wenn es keine Dokumentation gibt, die garantiert nicht verpasst, sieht das ehrlich gesagt nach dem erschreckendsten Weg aus, das Gleichgewicht zu finden möglich.

Micheal Johnson
quelle
0

Ich weiß, dass es viele Antworten gibt, aber ein weiterer Grund dafür charge(0)ist, dass ein einfacher Tippfehler dazu charge(9)führt, dass sich der Kontostand Ihres Kunden jedes Mal verringert, wenn Sie den Kontostand abrufen möchten. Wenn Sie über gute Unit-Tests verfügen, können Sie dieses Risiko möglicherweise mindern. Wenn Sie jedoch nicht bei jedem Anruf fleißig vorgehen, chargekann dies zu einem Missgeschick führen.

Shaz
quelle
-2

Ich möchte einen speziellen Fall erwähnen, in dem es sinnvoll wäre , weniger, mehr Mehrzweckmethoden zu haben: Wenn es viel Polymorphismus gibt, das heißt, viele Implementierungen dieser Schnittstelle ; Insbesondere, wenn diese Implementierungen in separat entwickeltem Code enthalten sind, der nicht synchron aktualisiert werden kann (die Schnittstelle wird durch eine Bibliothek definiert).

In diesem Fall ist die Vereinfachung des Schreibens jeder Implementierung weitaus wertvoller als die Klarheit der Verwendung, da erstere Vertragsverletzungsfehler vermeidet (die beiden Methoden sind nicht miteinander vereinbar), während letztere nur die Lesbarkeit beeinträchtigen, was möglich ist durch eine Hilfsfunktion oder eine Superklasse-Methode wiederhergestellt werden, die getBalanceim Sinne voncharge .

(Dies ist ein Entwurfsmuster, für das ich mich nicht an einen bestimmten Namen erinnere: Definieren einer komplexen aufruferfreundlichen Schnittstelle im Sinne einer minimalen implementierungsfreundlichen. In Classic Mac OS wurde die minimale Schnittstelle für Zeichenoperationen als "Engpass" bezeichnet.) Dies scheint jedoch kein populärer Begriff zu sein.)

Ist dies nicht der Fall (es gibt nur wenige oder genau eine Implementierung) charge(), ist es sinnvoll , die Methoden der Übersichtlichkeit halber zu trennen und das für Gebühren ungleich Null relevante Verhalten einfach zu addieren .

Kevin Reid
quelle
1
In der Tat hatte ich Fälle, in denen die minimalere Schnittstelle gewählt wurde, um das Testen der vollständigen Abdeckung zu vereinfachen. Dies ist jedoch nicht unbedingt für den Benutzer der Klasse sichtbar. Beispielsweise können Einfügen , Anhängen und Löschen einfache Wrapper eines allgemeinen Ersetzens sein, bei dem alle Steuerpfade gut untersucht und getestet wurden.
JDługosz
Ich habe eine solche API gesehen. Der Lua 5.1+ Allokator verwendet es. Sie bieten eine Funktion, und basierend auf den Parametern es geht, zu entscheiden , ob es neue Speicher, ausplanen Speicher oder zuzuweisen versucht Umspeichern Speicher aus alten Speicher. Es ist schmerzhaft , solche Funktionen zu schreiben. Mir wäre es viel lieber, wenn Lua Ihnen gerade zwei Funktionen gegeben hätte, eine für die Zuordnung und eine für die Freigabe.
Nicol Bolas
@NicolBolas: Und keine zur Umverteilung? Wie auch immer, dieser hat den zusätzlichen "Vorteil", dass er fast derselbe ist wie reallocmanchmal auf einigen Systemen.
Deduplizierer
@ Deduplicator: Zuordnung vs. Neuzuordnung ist ... OK. Es ist nicht großartig, sie in dieselbe Funktion zu versetzen, aber es ist nicht annähernd so schlimm wie die Freigabe in dieselbe Funktion. Es ist auch eine einfache Unterscheidung.
Nicol Bolas
Ich würde sagen, dass die Verknüpfung von Freigabe und (Neu-) Zuweisung ein besonders schlechtes Beispiel für diese Art von Schnittstelle ist. (Deallocation hat einen ganz anderen Vertrag: Es führt nicht zu einem gültigen Zeiger.)
Kevin Reid