Bereinigen Sie Code und hybride Objekte und Feature-Neid

10

Daher habe ich kürzlich einige wichtige Änderungen an meinem Code vorgenommen. Eines der wichtigsten Dinge, die ich versuchte, war, meine Klassen in Datenobjekte und Arbeitsobjekte aufzuteilen. Dies wurde unter anderem durch diesen Abschnitt von Clean Code inspiriert :

Hybriden

Diese Verwirrung führt manchmal zu unglücklichen hybriden Datenstrukturen, die halb Objekt- und halb Datenstruktur sind. Sie haben Funktionen, die wichtige Dinge tun, und sie haben entweder öffentliche Variablen oder öffentliche Zugriffsmethoden und Mutatoren, die die privaten Variablen in jeder Hinsicht öffentlich machen und andere externe Funktionen dazu verleiten, diese Variablen so zu verwenden, wie ein prozedurales Programm a verwenden würde Datenstruktur.

Solche Hybride erschweren das Hinzufügen neuer Funktionen, aber auch das Hinzufügen neuer Datenstrukturen. Sie sind die schlimmsten beider Welten. Vermeiden Sie es, sie zu erstellen. Sie weisen auf ein durcheinandergebrachtes Design hin, dessen Autoren sich nicht sicher sind oder gar nicht wissen, ob sie Schutz vor Funktionen oder Typen benötigen.

Kürzlich habe ich mir den Code für eines meiner Worker-Objekte angesehen (das zufällig das Besuchermuster implementiert ) und Folgendes gesehen:

@Override
public void visit(MarketTrade trade) {
    this.data.handleTrade(trade);
    updateRun(trade);
}

private void updateRun(MarketTrade newTrade) {
    if(this.data.getLastAggressor() != newTrade.getAggressor()) {
        this.data.setRunLength(0);
        this.data.setLastAggressor(newTrade.getAggressor());
    }
    this.data.setRunLength(this.data.getRunLength() + newTrade.getLots());
}

Ich sagte mir sofort: "Feature Neid! Diese Logik sollte in der DataKlasse sein - speziell in der handleTradeMethode. handleTradeUnd updateRunsollte immer zusammen passieren." Aber dann dachte ich: "Die Datenklasse ist nur eine publicDatenstruktur. Wenn ich damit anfange, wird es ein Hybridobjekt!"

Was ist besser und warum? Wie entscheiden Sie, was zu tun ist?

durron597
quelle
2
Warum muss 'Daten' überhaupt eine Datenstruktur sein? Es hat klares Verhalten. Schalten Sie einfach alle Getter und Setter aus, damit kein Objekt den internen Zustand manipulieren kann.
Cormac Mulhall

Antworten:

9

Der von Ihnen zitierte Text hat gute Ratschläge, obwohl ich "Datenstrukturen" durch "Datensätze" ersetzen werde, vorausgesetzt, dass so etwas wie Strukturen gemeint ist. Datensätze sind nur dumme Datenaggregationen. Während sie veränderlich sein können (und daher in einer Denkweise der funktionalen Programmierung zustandsbehaftet sind), haben sie keinen internen Zustand, keine Invarianten, die geschützt werden müssen. Es ist völlig gültig, einem Datensatz Vorgänge hinzuzufügen, die seine Verwendung erleichtern.

Zum Beispiel können wir argumentieren, dass ein 3D-Vektor eine dumme Aufzeichnung ist. Dies sollte uns jedoch nicht daran hindern, eine Methode wie addhinzuzufügen, die das Hinzufügen von Vektoren erleichtert. Das Hinzufügen von Verhalten verwandelt einen (nicht so dummen) Datensatz nicht in einen Hybrid.

Diese Linie wird überschritten, wenn die öffentliche Schnittstelle eines Objekts es uns ermöglicht, die Kapselung zu unterbrechen: Es gibt einige Interna, auf die wir direkt zugreifen können, wodurch das Objekt in einen ungültigen Zustand versetzt wird. Es scheint mir, dass Dataes einen Zustand gibt und dass er in einen ungültigen Zustand gebracht werden kann:

  • Nach der Abwicklung eines Handels wird der letzte Angreifer möglicherweise nicht aktualisiert.
  • Der letzte Angreifer kann aktualisiert werden, auch wenn kein neuer Handel stattgefunden hat.
  • Die Lauflänge behält möglicherweise ihren alten Wert bei, selbst wenn der Angreifer aktualisiert wurde.
  • usw.

Wenn ein Status für Ihre Daten gültig ist, ist alles in Ordnung mit Ihrem Code, und Sie können weitermachen. Andernfalls: Die DataKlasse ist für ihre eigene Datenkonsistenz verantwortlich. Wenn die Abwicklung eines Handels immer die Aktualisierung des Angreifers beinhaltet, muss dieses Verhalten Teil der DataKlasse sein. Wenn zum Ändern des Angreifers die Lauflänge auf Null gesetzt werden muss, muss dieses Verhalten Teil der DataKlasse sein. Datawar nie ein dummer Rekord. Sie haben es bereits zu einem Hybrid gemacht, indem Sie öffentliche Setter hinzugefügt haben.

Es gibt ein Szenario, in dem Sie erwägen können, diese strengen Verantwortlichkeiten zu lockern: Wenn DataIhr Projekt privat ist und daher nicht Teil einer öffentlichen Schnittstelle ist, können Sie dennoch die ordnungsgemäße Verwendung der Klasse sicherstellen. Dies überträgt jedoch die Verantwortung für die Aufrechterhaltung der DataKonsistenz im gesamten Code, anstatt sie an einem zentralen Ort zu sammeln.

Ich habe kürzlich eine Antwort zur Kapselung geschrieben , die ausführlicher darauf eingeht, was Kapselung ist und wie Sie sie sicherstellen können.

amon
quelle
5

Die Tatsache, dass handleTrade()und updateRun()immer zusammen geschehen (und die zweite Methode tatsächlich beim Besucher liegt und mehrere andere Methoden beim Datenobjekt aufruft), riecht nach zeitlicher Kopplung . Dies bedeutet , dass Sie müssen Methoden in einer bestimmten Reihenfolge nennen, und ich wurde , dass das Aufrufen von Methoden aus , um erraten etwas im schlimmsten Fall brechen, oder nicht ein sinnvolles Ergebnis bestenfalls zu liefern. Nicht gut.

Normalerweise besteht die richtige Methode zum Umgestalten dieser Abhängigkeit darin, dass jede Methode ein Ergebnis zurückgibt, das entweder in die nächste Methode eingespeist oder direkt bearbeitet werden kann.

Alter Code:

MyObject x = ...;
x.actionOne();
x.actionTwo();
String result = x.actionThree();

Neuer Code:

MyObject x = ...;
OneResult r1 = x.actionOne();
TwoResult r2 = r1.actionTwo();
String result = r2.actionThree();

Dies hat mehrere Vorteile:

  • Es verschiebt separate Anliegen in separate Objekte ( SRP ).
  • Es beseitigt die zeitliche Kopplung: Es ist unmöglich, Methoden außerhalb der Reihenfolge aufzurufen, und die Methodensignaturen bieten eine implizite Dokumentation zum Aufrufen. Haben Sie sich jemals die Dokumentation angesehen, das gewünschte Objekt gesehen und rückwärts gearbeitet? Ich möchte Objekt Z. Aber ich brauche ein Y, um ein Z zu bekommen. Um ein Y zu bekommen, brauche ich ein X. Aha! Ich habe ein W, das erforderlich ist, um ein X zu erhalten. Verketten Sie alles miteinander, und Ihr W kann jetzt verwendet werden, um ein Z zu erhalten.
  • Das Aufteilen solcher Objekte macht sie mit größerer Wahrscheinlichkeit unveränderlich, was zahlreiche Vorteile bietet, die über den Rahmen dieser Frage hinausgehen. Das schnelle Mitnehmen ist, dass unveränderliche Objekte dazu neigen, zu Code zu führen, der sicherer ist.

quelle
Es gibt keine zeitliche Kopplung zwischen diesen beiden Methodenaufrufen. Tauschen Sie ihre Reihenfolge und das Verhalten ändert sich nicht.
Durron597
1
Beim Lesen der Frage dachte ich zunächst auch an sequentielle / zeitliche Kopplung, bemerkte dann aber, dass die updateRunMethode privat war . Das Vermeiden der sequentiellen Kopplung ist ein guter Rat, gilt jedoch nur für API-Design / öffentliche Schnittstellen und nicht für Implementierungsdetails. Die eigentliche Frage scheint zu sein, ob updateRunsie sich im Besucher oder in der Datenklasse befinden soll, und ich sehe nicht, wie diese Antwort dieses Problem angeht.
Amon
Die Sichtbarkeit von updateRunist irrelevant. Wichtig ist, dass die Implementierung this.datain der Frage nicht vorhanden ist und das Objekt vom Besucherobjekt manipuliert wird.
Wenn überhaupt, ist die Tatsache, dass dieser Besucher einfach eine Reihe von Setzern anruft und nichts verarbeitet, ein Grund dafür, dass keine zeitliche Kopplung vorliegt. Es spielt wahrscheinlich keine Rolle, in welcher Reihenfolge die Setter aufgerufen werden.
0

Aus meiner Sicht sollte eine Klasse "Werte für state (Mitgliedsvariablen) und Verhaltensimplementierungen (Mitgliedsfunktionen, Methoden)" enthalten.

Die "unglücklichen hybriden Datenstrukturen" entstehen, wenn Sie Klassenstatus-Mitgliedsvariablen (oder deren Getter / Setter) öffentlich machen, die nicht öffentlich sein sollten.

Ich sehe also keine Notwendigkeit, separate Klassen für Datendatenobjekte und Arbeitsobjekte zu haben.

Sie sollten in der Lage sein, Statusmitgliedsvariablen nicht öffentlich zu halten (Ihre Datenbankebene sollte in der Lage sein, nicht öffentliche Mitgliedsvariablen zu verarbeiten).

Ein Feature-Neid ist eine Klasse, die übermäßig Methoden einer anderen Klasse verwendet. Siehe Code_smell . Eine Klasse mit Methoden und Status würde dies beseitigen.

k3b
quelle