Command Query Separation

[DrPils]

Functions should either do something or answer something, but not both. Either your function should change the state of an object, or it should return some information about that object.

Robert C. Martin

Heist also Methoden die den Zustand eines Objektes verändern returnen void, Queries returnen den jeweiligen Typ.
Finde ich erstmal gut. So ist die Zuständigkeit der Methode immer klar und es gibt keine verwirrung was man mit dem returnten Objekt anfangen soll.
Wenn eine Methode den Zustand verändert und etwas returnt, macht sie auch zwei Dinge.
Wirft meine save Methode eine Exception wurde wohl nichts gespeichert, dafür brauche ich keinen boolean.

Nun sehen das die Java API und der Rest der Welt anders.

Bei Collections gibt die add(Object o) true zurück falls das Objekt hinzugefügt wurde. Wieso nicht einfach eine Exception werfen? Tatsächlich wird ja eine IllegalStateException geworfen falls dies der fall ist. Was den boolean redundant macht.
Bei CRUD Methoden sieht man es auch oft. Null als pseudo boolean bei read, das entfernte Objekt zurückgeben bei delete, usw ...

Ich stehe jetzt ein bisschen auf dem Schlauch, ob ich mich nach Onkel Bob oder dem Rest richten soll. Ersteres leuchtet mir eher ein.
Wie seht ihr das?

 

[mihe7]

Nun sehen das die Java API und der Rest der Welt anders.

Vorab: nein.

Bei Collections gibt die add(Object o) true zurück falls das Objekt hinzugefügt wurde.

Das ist auch in Ordnung: die Methode gibt das Ergebnis des Vorgangs und keine Information über das Objekt zurück.

Das große Problem ist m. E. auch weniger, dass Methoden etwas zurückgeben, sondern wenn Methoden so tun als würden sie eine Abfrage beantworten, im Hintergrund aber gleichzeitig etwas ändern.

Ich hatte mal so einen Fall, der mich den letzten Nerv gekostet hat: eine Methode zur Abfrage aus einer DB. Der Witz: die Methode fügte Datensätze hinzu, die zuvor in einer globalen Variable gespeichert wurden. Nur das Beste vom Besten...

Da der Code insgesamt von dieser Qualität war, habe ich im Rahmen der Fehlersuche bzgl. eines gnz anderen Problems angefangen, den Code zu bereinigen, um überhaupt noch irgendwie durchzublicken. Da fanden sich dann lustige Aufrufe der Form getXXX(), ohne dass das Ergebnis verwendet worden wäre. Den Schwachsinn erstmal rausgeschmissen. Ergebnis: Programm funktioniert plötzlich an ganz anderen Stellen hinten und vorne nicht mehr. Da suchst Du Dich dumm und dämlich

Wieso nicht einfach eine Exception werfen?

Weil es keine Ausnahme sein muss, dass ein Objekt nicht in eine Collection aufgenommen wurde. Ein Set z. B. bildet eine Menge ab. Es ist völlig ok, dort mehrere gleiche Objekte hineinzustecken (eine Menge aus gleichen Objekten zu bilden), es wird aber nur einmal hinzugefügt.

 

[LimDul]

Man muss - @mihe7 hat es ja schon beleuchtet - unterscheiden zwischen Konzepten und der Umsetzung.

Konzeptionell sollte man folgende Dinge nicht vermischen:
* Eine Beauskunftung
* Eine Änderung

Das heißt, wenn ich eine Methode habe, die mir eine Auskunft über irgendwas gibt, dann darf diese Methode nicht den Zustand ändern. Sprich - solange ich nur Methoden zur Beauskunftung aufrufe, bekomme ich egal in welcher Reihenfolge das gleich Ergebnis. Das kann ein Return Parameter sein, kann aber auch komplett anders gelöst sein technisch, wo die Auskunft dann hingeht. Aber wichtig ist, der Zustand des Objektes bleibt gleich.

Anders sieht es aus, wenn ich eine Operation/Änderung durchführe. Dann darf sich der Zustand des Objektes ändern - das muss dann aber auch klar ersichtlich sein. Und natürlich hat eine Änderung durchaus ein Ergebnis. Das kann zurückgegeben werden oder kann woanders hin geschrieben werden etc. Wichtig ist halt, dem Aufrufer ist klar, er ändert was am Zustand.

Man darf nur nicht den Fehler machen diese Konzepte 1:1 als Programmierkonzepte zu sehen, dass ist falsch. Es sind abstrakte Konzepte, wo es keine eindeutige Umsetzung gibt. Sie spiegeln sich eher in Namenskonventionen und Java-Doc wieder den technischen Methodenbeschreibungen. So ist eine getXXX / readXXX Methode eine Beauskunftung. Eine writeXXX/setXXX Methode eine Änderung.

 

[kneitzel]

Ich werfe einfach mal meine Signatur ein.
"Clean code is simple and direct. Clean code reads like well-written prose..."

Das sagt diesbezüglich auch einiges aus. "getSomething" -> das sagt ja klar, was es macht. Da ist dann jede Änderung eines Zustandes nicht in der Aussage enthalten. Ebenso "changeSomething" - was wird da zurück gegeben?

Das würde sonst zu Methoden führen a.la. "doSomethingAndGetSomethingElse"?

 

[Neumi5694]

Ein praktisches Beispiel: Ganz schlimm wäre z.B. die Flächenberechnung eines Rechtecks, die im selben Moment die Maße ändert. Ruf die zwei mal hintereinander auf und staune.

Immer lässt sich das Vermischen nicht vermeiden (put bei einer Map liefert den vorher unter diesem Key gespeicherten Wert), es geht halt darum, dass man es nicht machen "sollte", wo es sich vermeiden lässt.
Andererseits ergibt es durchaus Sinn, mehrere Schritte in eine Methode zusammenzufassen, anstatt Spaghetticode zu haben. eine Methode "getThisAndDoThat" ist also durchaus sinnvoll, wenn sie gut benannt, beschrieben und dokumentiert ist, besser wäre in dem Fall aber die Benennung "doThisAndGetTheResult". Grundsätzlich sollte von der Benennung her eine getX Methode nichts 'machen'.

 

[DrPils]

Ich werfe einfach mal meine Signatur ein.
"Clean code is simple and direct. Clean code reads like well-written prose..."

Das sagt diesbezüglich auch einiges aus. "getSomething" -> das sagt ja klar, was es macht. Da ist dann jede Änderung eines Zustandes nicht in der Aussage enthalten. Ebenso "changeSomething" - was wird da zurück gegeben?

Das würde sonst zu Methoden führen a.la. "doSomethingAndGetSomethingElse"?

Genau darum geht es mir ja.
Was sagt mir ein Set.add()? Ersteinmal dass es etwas tut, aber es gibt ein boolean zurueck, also sollte es mir eine Frage beantworten. Wenn ich eine Frage stelle sollte sich nicht der State aendern.
Wieso muss eine Set.add() true zurückgeben? Ich fuege was hinzu, entweder war es schon im Set oder nicht. Das ergebnis ist in beiden fällen das gleiche.

 

[LimDul]

Genau darum geht es mir ja.
Was sagt mir ein Set.add()? Ersteinmal dass es etwas tut, aber es gibt ein boolean zurueck, also sollte es mir eine Frage beantworten. Wenn ich eine Frage stelle sollte sich nicht der State aendern.
Wieso muss eine Set.add() true zurückgeben? Ich fuege was hinzu, entweder war es schon im Set oder nicht. Das ergebnis ist in beiden fällen das gleiche.

Der Aufrufer möchte oftmals schon wissen, ob eine Änderung erfolgreich war. Und es ist keine Beauskunftung. Du bekommst das Ergebnis einer Operation zurück - keine Auskunft. Ich kann keine Beschreibung formulieren, die dieses Ergebnis in irgendeiner Form als Beauskunftung bezeichnet und noch Sinn ergibt.

Welche Frage die du als abstrakte Auskunft an das Objekt stellst, beantwortet den der Rückgabe-Wert von add? Bitte als allgemeine abstrakte Auskunft formulieren ohne Bezug auf die durchgeführte Operation.

 

[Neumi5694]

Genau darum geht es mir ja.
Was sagt mir ein Set.add()? Ersteinmal dass es etwas tut, aber es gibt ein boolean zurueck, also sollte es mir eine Frage beantworten. Wenn ich eine Frage stelle sollte sich nicht der State aendern.
Wieso muss eine Set.add() true zurückgeben? Ich fuege was hinzu, entweder war es schon im Set oder nicht. Das ergebnis ist in beiden fällen das gleiche.

Du hast keine Frage gestellt, was wäre denn deine Frage in dem Fall?
Du hast etwas geändert und als Rückmeldung hast du gekriegt, ob die Änderung erfolgreich war. Das ist absolut ok.

Edit: Würde man es nicht so machen, so müsste der Aufruf sein.

set.add(x);
boolean didAdd = set.wasLastOperationOk(), //klappt natürlich nur, wenn kein zweiter Thread zwischen diesen beiden Befehlen was anderes macht.

Macht das den Code einfacher? Lesbararer? Auf jeden Fall kriegst du so ein Problem mit Threads. Was, wenn (zeitlich) nach dem add und vor dem wasLastOperationOk noch was anderes passiert?

 

[mihe7]

Ich fuege was hinzu, entweder war es schon im Set oder nicht. Das ergebnis ist in beiden fällen das gleiche.

Das ist ja gerade der Unterschied:

die Methode gibt das Ergebnis des Vorgangs und keine Information über das Objekt zurück

Das true/false sagt hier eben nichts über die Liste aus. Vergleiche das nochmal mit

Either your function should change the state of an object, or it should return some information about that object.

Hier wird eben keine Information über das Objekt (Liste) zurückgegeben, sondern einfach nur, ob im Rahmen der add-Ausführung die Liste tatsächlich verändert wurde.

 

[DrPils]

Du hast keine Frage gestellt, was wäre denn deine Frage in dem Fall?

Ja aber der boolean signalisiert doch dass ich hier eine Frage beantwortet bekomme.

Würde man es nicht so machen, so müsste der Aufruf sein.

Wieso denn? Wenn die letzte Operation nicht ok war wurde ja eine Exception geworfen. Anders ist das Objekt so oder so drinnen, ob es schon drinnen war oder nicht intressiert mich nicht.

Welche Frage die du als abstrakte Auskunft an das Objekt stellst, beantwortet den der Rückgabe-Wert von add?

Keine, genau das ist doch mein problem

 

[kneitzel]

Ja aber der boolean signalisiert doch dass ich hier eine Frage beantwortet bekomme.

Nein, hast Du einmal in einem Umfeld gearbeitet, in dem es um kritische Dinge geht?
Da wird ein Befehl gegeben und dann kommt die Bestätigung:

Also Feuerwehr: "Angrifftrumm unter PA in den 2. Stock vor und Abluftöffnung schaffen".
Da kommt dann erst noch eine Wiederholung, die den richtigen Transfer sicher stellt. Braucht man hier nicht, klar.
Aber dann kommt nach Erfüllung des Auftrags die Rückmeldung: "Abluftöffnung geschaffen".

Das ist also nur die Bestätigung, wie das Ergebnis ist. Es ist also ein Ergebnis des ausgeführten Befehls.

 

[DrPils]

Nein, hast Du einmal in einem Umfeld gearbeitet, in dem es um kritische Dinge geht?
Da wird ein Befehl gegeben und dann kommt die Bestätigung:

Also Feuerwehr: "Angrifftrumm unter PA in den 2. Stock vor und Abluftöffnung schaffen".
Da kommt dann erst noch eine Wiederholung, die den richtigen Transfer sicher stellt. Braucht man hier nicht, klar.
Aber dann kommt nach Erfüllung des Auftrags die Rückmeldung: "Abluftöffnung geschaffen".

Das ist also nur die Bestätigung, wie das Ergebnis ist. Es ist also ein Ergebnis des ausgeführten Befehls.

Und wenn vor dem Feuerwehrmann ein anderer den Auftrag schon erledigt hat und der Feuerwehrmann jetzt antwortet: "Auftrag konnte nicht erfolgreich abgeschlossen werden"?
Das waere ja der fall bei dem Set

 

[Neumi5694]

Ja aber der boolean signalisiert doch dass ich hier eine Frage beantwortet bekomme.

Wieso denn? Wenn die letzte Operation nicht ok war wurde ja eine Exception geworfen. Anders ist das Objekt so oder so drinnen, ob es schon drinnen war oder nicht intressiert mich nicht.

Eine Exception? Wieso? Du wolltest doch nut ein Element hinzufügen, der Sinn und Zweck eines Sets ist, dass das nicht passiert, wenn das Element schon drin ist. Eine Exception würde das Ganze ad absurdum führen.
Aber benennen wir die Methode halt um, wenn dir das lieber ist nach "didTheLastAddOperationChangeTheSet()".
Wäre das sinnvoller?

Im Fall des Feuerwehrmanns wäre ein Enum als Rückgabewert sinnvoller: Erledigt/HabIchSchon/WillNicht/GingDaneben/SagsHaraldDerHatZeit.
Wichtig ist nur, DASS du eine Rückmeldung kriegst, damit du weißt, wie du darauf reagieren kannst.

 

[kneitzel]

Und wenn vor dem Feuerwehrmann ein anderer den Auftrag schon erledigt hat und der Feuerwehrmann jetzt antwortet: "Auftrag konnte nicht erfolgreich abgeschlossen werden"?
Das waere ja der fall bei dem Set

Das ist doch egal. Was für ein Rückgabewert wann wie gegeben wird muss man bei der Entwicklung sehen. Es geht erst einmal nur darum, dass es eben bei einem Befehl eine Rückmeldung über die Ausführung gibt.

 

[DrPils]

Ich verstehe nicht wieso die Rückmeldung von Bedeutung ist. Der Sinn von add ist es ja was in das Set zu stecken. Ist es schon im Set dann eben nicht, aber ich kann mir sicher sein, dass es drinnen ist.

Ebenso gut könnte ich jetzt auch anfangen bei Settern einen bool zurückgeben, true nur falls der Wert sich tatsächlich geändert hat

 

[mihe7]

Collection#add(E e) besagt nicht, dass ein Element zur Collection hinzugefügt wird sondern lediglich, dass die Methode sicherstellt, dass nach dem Aufruf das angegebene Element e in der Collection enthalten ist.

Wenn diese Nachbedingung nicht erfüllt werden kann, tritt eine Exception auf. Wenn ich z. B. auf einer Menge ein add(a) aufrufe und die Methode nicht dafür sorgen kann, dass die Menge spätestens nach dem Aufruf das a enthält, dann hat eine Exception aufzutreten.

D. h. aber auch, dass die Menge zum Zeitpunkt des add-Aufrufs das a bereits enthalten haben könnte, denn auch dann ist sichergestellt, dass nach dem add-Aufruf das a in der Menge enthalten ist.

Es kann aber für Algorithmen, die eine Collection verwenden, von Interesse sein, ob sich die Collection durch den add-Aufruf auch tatsächlich verändert hat.

 

[Neumi5694]

Ich verstehe nicht wieso die Rückmeldung von Bedeutung ist. Der Sinn von add ist es ja was in das Set zu stecken. Ist es schon im Set dann eben nicht, aber ich kann mir sicher sein, dass es drinnen ist.

Ebenso gut könnte ich jetzt auch anfangen bei Settern einen bool zurückgeben, true nur falls der Wert sich tatsächlich geändert hat

Niemand hindert dich daran, bei der put-Methode in einer Map passiert genau das.

Falls das Ganze absolut verboten wäre, wäre es noch nicht mal möglich. Das ist nur eine Grundsatzregel.

Schau dir bei der Gelegenheit mal die computeIfAbsent-Methode einer Map an (oder putIfAbsent für einfache Werte).
Da passiert im Grunde folgendes:

//Aufruf:
    T e = map.computeIfAbsentPseudoCode(key, newValue);

//Implementierung
    T computeIfAbsentPseudoCode(E key, Supplier<T> newValue) {
      if (!this.containsKey(key)) {
          this.put(key, newValue.get());
      }
      return this.getKey();
  }

Das ist eine Hilfsmethode, die die Map gegebenenfalls verändert und dir dann das Element liefert, das entweder schon unter dem Key gespeichert war oder gerade neu erzeugt wurde und jetzt unter dem Key gespeichert ist.

Würden wir uns ganz strikt an das Paradigma halten, so müsstest du diesen Code jedesmal wo benötigt ins Programm einfügen, dadurch wäre das Programm viel schwerer zu warten und schlechter lesbar.

 

[kneitzel]

Ob die Rückmeldung von Bedeutung ist oder nicht hängt doch vom Anwendungsfall ab. Es gibt Situationen, in denen ist es egal. Und es gibt Situationen, in denen ist es nicht egal.

Wenn Du aber nun den Anwendungsfall nicht kennst (z.B. bei einer universellen Library), dann baut man es universell. Daher ist es in dem Java Framework so implementiert.

In der eigenen App kennst Du den Anwendungsfall. Und wenn Du es da nicht brauchst, dann hast Du es in der Regel auch schlicht nicht.

 

[DrPils]

Niemand hindert dich daran, bei der put-Methode in einer Map passiert genau das.

Falls das Ganze absolut verboten wäre, wäre es noch nicht mal möglich. Das ist nur eine Grundsatzregel.

Lass doch mal bitte die Strohmaenner sein

 

[Neumi5694]

Lass doch mal bitte die Strohmaenner sein

*verwirrtdreinschau* was?

 

[kneitzel]

Nehmen wir doch einfach irgend eine andere Methode, die aktiv etwas macht und etwas zurück gibt. Eine add() Methode, die dann etwas addiert und eine neue Instanz mit dem Ergebnis zurück gibt wäre so ein typisches Beispiel:

Bruch a = new Bruch (1, 3);
Bruch b = new Bruch (3, 4);
Bruch c = a.add(b);

Das add addiert zwei Brüche. Das ist eine ganz aktive Sache. Und das Ergebnis der Aktion wird zurück gegeben.

Das wäre ein Beispiel, bei dem das Ergebnis immer von Interesse sein dürfte.

 

[DrPils]

Das wäre ein Beispiel, bei dem das Ergebnis immer von Interesse sein dürfte.

Ok bei dem Beispiel gibt es tatsächlich nichts zu hinterfragen .