Verstößt das Hinzufügen eines Rückgabetyps zu einer Aktualisierungsmethode gegen das Prinzip der Einzelverantwortung?

Ich habe eine Methode, die Mitarbeiterdaten in der Datenbank aktualisiert. Die EmployeeKlasse ist unveränderlich. "Aktualisieren" des Objekts bedeutet also, tatsächlich ein neues Objekt zu instanziieren.

Ich möchte, dass die UpdateMethode eine neue Instanz von Employeemit den aktualisierten Daten zurückgibt, aber seitdem kann ich sagen, dass die Verantwortung der Methode darin besteht , die Mitarbeiterdaten zu aktualisieren und ein neues EmployeeObjekt aus der Datenbank abzurufen. Verstößt dies gegen das Prinzip der einfachen Verantwortung ?

Der DB-Datensatz selbst wird aktualisiert. Dann wird ein neues Objekt instanziiert, um diesen Datensatz darzustellen.

Wie bei jeder Regel denke ich, ist es hier wichtig, den Zweck der Regel, den Geist, zu betrachten und nicht zu analysieren, wie die Regel in einem Lehrbuch formuliert wurde und wie dies auf diesen Fall angewendet werden kann. Wir müssen uns dem nicht wie Anwälte nähern. Der Zweck der Regeln ist es, uns zu helfen, bessere Programme zu schreiben. Es ist nicht so, dass der Zweck des Programmschreibens darin besteht, die Regeln einzuhalten.

Der Zweck der Einzelverantwortungsregel besteht darin, Programme verständlicher zu machen und zu warten, indem jede Funktion eine in sich geschlossene, kohärente Aufgabe ausführt.

Ich habe zum Beispiel einmal eine Funktion geschrieben, die ich so etwas wie "checkOrderStatus" nannte, die feststellte, ob eine Bestellung aussteht, versandt oder nachbestellt wurde, und einen Code zurückgab, der angibt, welche. Dann kam ein anderer Programmierer und änderte diese Funktion, um auch die verfügbare Menge zu aktualisieren, wenn die Bestellung versandt wurde. Dies verstieß erheblich gegen das Prinzip der einheitlichen Verantwortung. Ein anderer Programmierer, der diesen Code später liest, würde den Funktionsnamen sehen, sehen, wie der Rückgabewert verwendet wurde, und er könnte durchaus nie den Verdacht haben, dass eine Datenbankaktualisierung durchgeführt wurde. Jemand, der den Bestellstatus abrufen musste, ohne die verfügbare Menge zu aktualisieren, befand sich in einer schwierigen Situation: Sollte er eine neue Funktion schreiben, die den Bestellstatus-Teil dupliziert? Fügen Sie ein Flag hinzu, um zu bestimmen, ob das DB-Update durchgeführt werden soll. Etc.

Andererseits würde ich nicht auswählen, was "zwei Dinge" ausmacht. Ich habe kürzlich eine Funktion geschrieben, die Kundeninformationen von unserem System an das System unseres Kunden sendet. Diese Funktion führt eine Neuformatierung der Daten durch, um ihre Anforderungen zu erfüllen. Zum Beispiel haben wir einige Felder in unserer Datenbank, die möglicherweise null sind, aber keine Nullen zulassen. Deshalb müssen wir einen Dummy-Text "nicht angegeben" eingeben, oder ich vergesse die genauen Wörter. Diese Funktion bewirkt vermutlich zwei Dinge: Formatieren Sie die Daten neu UND senden Sie sie. Aber ich habe dies ganz bewusst in eine einzige Funktion gestellt, anstatt "neu formatieren" und "senden" zu müssen, weil ich niemals ohne Neuformatierung senden möchte. Ich möchte nicht, dass jemand einen neuen Anruf schreibt und nicht merkt, dass er neu formatieren und dann senden muss.

Aktualisieren Sie in Ihrem Fall die Datenbank und geben Sie ein Bild des geschriebenen Datensatzes zurück. Dies scheint zwei Dinge zu sein, die logisch und unvermeidlich zusammenpassen könnten. Ich kenne die Details Ihrer Bewerbung nicht und kann daher nicht definitiv sagen, ob dies eine gute Idee ist oder nicht, aber es klingt plausibel.

Wenn Sie ein Objekt im Speicher erstellen, das alle Daten für den Datensatz enthält, die Datenbankaufrufe ausführen, um dies zu schreiben, und dann das Objekt zurückgeben, ist dies sehr sinnvoll. Sie haben den Gegenstand in Ihren Händen. Warum nicht einfach zurückgeben? Wenn Sie das Objekt nicht zurückgegeben haben, wie würde der Anrufer es erhalten? Müsste er die Datenbank lesen, um das Objekt zu erhalten, das Sie gerade geschrieben haben? Das scheint ziemlich ineffizient zu sein. Wie würde er die Aufzeichnung finden? Kennen Sie den Primärschlüssel? Wenn jemand erklärt, dass es "legal" ist, dass die Schreibfunktion den Primärschlüssel zurückgibt, damit Sie den Datensatz erneut lesen können, warum nicht einfach den gesamten Datensatz zurückgeben, damit Sie dies nicht tun müssen? Was ist der Unterschied?

Wenn das Erstellen des Objekts eine Menge Arbeit ist, die sich vom Schreiben des Datenbankdatensatzes deutlich unterscheidet, und ein Aufrufer den Schreibvorgang ausführen, das Objekt jedoch nicht erstellen möchte, kann dies eine Verschwendung sein. Wenn ein Aufrufer das Objekt haben möchte, aber nicht schreibt, müssen Sie einen anderen Weg angeben, um das Objekt zu erhalten. Dies kann bedeuten, dass redundanter Code geschrieben wird.

Aber ich denke, Szenario 1 ist wahrscheinlicher, also würde ich sagen, wahrscheinlich kein Problem.

Das Konzept der SRP besteht darin, Module daran zu hindern, zwei verschiedene Dinge zu tun, wenn sie einzeln ausgeführt werden, um eine bessere Wartung und weniger Spaghetti in der Zeit zu erzielen. Wie die SRP sagt "ein Grund zur Veränderung".

In Ihrem Fall, wenn Sie eine Routine hatten, die "aktualisiert und das aktualisierte Objekt zurückgibt", ändern Sie das Objekt immer noch einmal - mit einem Grund für die Änderung. Dass Sie das Objekt gleich wieder zurückgeben, ist weder hier noch da, Sie arbeiten immer noch an diesem einzelnen Objekt. Der Code ist für eine und nur eine Sache verantwortlich.

Bei der SRP geht es nicht wirklich darum, alle Vorgänge auf einen einzigen Anruf zu reduzieren, sondern darum, zu reduzieren, woran Sie arbeiten und wie Sie daran arbeiten. Ein einzelnes Update, das das aktualisierte Objekt zurückgibt, ist also in Ordnung.

Wie immer ist dies eine Frage des Grades. Die SRP sollte Sie daran hindern, eine Methode zu schreiben, die einen Datensatz aus einer externen Datenbank abruft, eine schnelle Fourier-Transformation durchführt und eine globale Statistikregistrierung mit dem Ergebnis aktualisiert. Ich denke, dass fast jeder zustimmen würde, dass diese Dinge mit verschiedenen Methoden gemacht werden sollten. Das Postulieren einer einzelnen Verantwortung für jede Methode ist einfach die wirtschaftlichste und einprägsamste Möglichkeit, dies zu verdeutlichen.

Am anderen Ende des Spektrums befinden sich Methoden, die Informationen über den Zustand eines Objekts liefern. Der typische isActivehat diese Informationen als seine einzige Verantwortung zu geben. Wahrscheinlich sind sich alle einig, dass dies in Ordnung ist.

Nun erweitern einige das Prinzip so weit, dass sie die Rückgabe eines Erfolgszeichens als eine andere Verantwortung betrachten als die Durchführung der Aktion, deren Erfolg gemeldet wird. Bei einer äußerst strengen Interpretation ist dies richtig, aber da die Alternative darin bestehen müsste, eine zweite Methode aufzurufen, um den Erfolgsstatus zu erhalten, was den Aufrufer kompliziert, können viele Programmierer die Erfolgscodes einer Methode mit Nebenwirkungen problemlos zurückgeben.

Die Rückgabe des neuen Objekts ist ein weiterer Schritt auf dem Weg zu mehreren Verantwortlichkeiten. Es ist ein wenig sinnvoller, den Anrufer zu einem zweiten Anruf für ein ganzes Objekt zu verpflichten, als einen zweiten Anruf zu tätigen, um festzustellen, ob der erste erfolgreich war oder nicht. Dennoch würden viele Programmierer in Betracht ziehen, das Ergebnis eines Updates einwandfrei zurückzugeben. Während dies als zwei leicht unterschiedliche Verantwortlichkeiten ausgelegt werden kann, ist es sicherlich nicht einer der ungeheuren Missbräuche, die das Prinzip anfangs inspiriert haben.

Verstößt es gegen das Prinzip der einheitlichen Verantwortung?

Nicht unbedingt. Wenn überhaupt, verstößt dies gegen das Prinzip der Trennung von Befehlen und Abfragen .

Die Verantwortung ist

Aktualisieren Sie die Mitarbeiterdaten

mit dem impliziten Verständnis, dass diese Verantwortung den Status der Operation umfasst; zB wenn die Operation fehlschlägt, wird eine Ausnahme ausgelöst, wenn sie erfolgreich ist, wird der Update-Mitarbeiter zurückgegeben, usw.

Auch dies ist alles eine Frage des Grades und der subjektiven Beurteilung.

Was ist also mit der Trennung von Befehlen und Abfragen?

Nun, dieses Prinzip existiert, aber die Rückgabe des Ergebnisses eines Updates ist in der Tat sehr verbreitet.

(1) Java Set#add(E)fügt das Element hinzu und gibt seine vorherige Aufnahme in die Menge zurück.

if (visited.add(nodeToVisit)) {
    // perform operation once
}

Dies ist effizienter als die CQS-Alternative, bei der möglicherweise zwei Suchvorgänge ausgeführt werden müssen.

if (!visited.contains(nodeToVisit)) {
    visited.add(nodeToVisit);
    // perform operation once
}

(2) Compare-and-Swap , Fetch-and-Add und Test-and-Set sind übliche Grundelemente, die die gleichzeitige Programmierung ermöglichen. Diese Muster treten häufig auf, von CPU-Anweisungen auf niedriger Ebene bis hin zu gleichzeitigen Sammlungen auf hoher Ebene.

Die einzige Verantwortung betrifft eine Klasse, die sich nicht aus mehr als einem Grund ändern muss.

Als Beispiel hat ein Mitarbeiter eine Liste von Telefonnummern. Wenn sich die Art und Weise, wie Sie mit Telefonnummern umgehen, ändert (Sie können Ländervorwahlen hinzufügen), sollte dies die Mitarbeiterklasse überhaupt nicht ändern.

Ich hätte nicht die Angestelltenklasse, die wissen müsste, wie es sich in der Datenbank speichert, da es sich dann aufgrund von Änderungen bei Angestellten und Änderungen bei der Speicherung von Daten ändern würde.

Ebenso sollte es in der Mitarbeiterklasse keine CalculateSalary-Methode geben. Ein Gehaltseigentum ist in Ordnung, aber die Berechnung von Steuern usw. sollte woanders erfolgen.

Aber dass die Update-Methode das zurückgibt, was sie gerade aktualisiert hat, ist in Ordnung.

Wrt. der spezielle Fall:

Employee Update(Employee, name, password, etc) (Eigentlich mit einem Builder, da ich viele Parameter habe).

Es scheint, dass die UpdateMethode einen vorhandenen Employeeals ersten Parameter zur Identifizierung (?) Des vorhandenen Mitarbeiters und eine Reihe von Parametern zur Änderung dieses Mitarbeiters verwendet.

Ich kann denken , dies könnte getan sauberer sein. Ich sehe zwei Fälle:

(a) Employee enthält tatsächlich eine Datenbank / eindeutige ID, anhand derer sie immer in der Datenbank identifiziert werden kann. (Das heißt, Sie müssen nicht den gesamten Datensatzwert festlegen, um ihn in der Datenbank zu finden.

In diesem Fall würde ich eine void Update(Employee obj)Methode bevorzugen , die nur den vorhandenen Datensatz nach ID findet und dann die Felder aus dem übergebenen Objekt aktualisiert. Oder vielleicht einvoid Update(ID, EmployeeBuilder values)

Eine Variation davon, die ich für nützlich hielt, besteht darin, nur eine void Put(Employee obj)Methode zu haben, die einfügt oder aktualisiert, abhängig davon, ob der Datensatz (nach ID) vorhanden ist.

(b) Der vollständige vorhandene Datensatz wird für DB - Lookup benötigt, in diesem Fall könnte es noch mehr Sinn zu haben: void Update(Employee existing, Employee newData).

Soweit ich hier sehen kann, würde ich in der Tat sagen, dass die Verantwortung für das Erstellen eines neuen Objekts (oder von Unterobjektwerten) zum Speichern und tatsächlichen Speichern orthogonal ist, sodass ich sie trennen würde.

Die erwähnten gleichzeitigen Anforderungen in anderen Antworten (atomares Setzen und Abrufen / Vergleichen usw.) waren in dem DB-Code, an dem ich bisher gearbeitet habe, kein Problem. Wenn ich mit einem DB spreche, denke ich, dass dies auf der Transaktionsebene normal gehandhabt werden sollte, nicht auf der Ebene der einzelnen Anweisungen. (Das soll nicht heißen, dass es vielleicht kein Design gibt, bei dem "atomar" Employee[existing data] Update(ID, Employee newData)keinen Sinn ergibt, aber bei DB-Zugriffen ist dies nicht das, was ich normalerweise sehe.)

Bisher reden alle hier über Klassen. Aber denken Sie darüber aus der Perspektive der Benutzeroberfläche nach.

Wenn eine Schnittstellenmethode einen Rückgabetyp und / oder eine implizite Zusage zum Rückgabewert deklariert, muss jede Implementierung das aktualisierte Objekt zurückgeben.

Sie können also fragen:

• Können Sie sich mögliche Implementierungen vorstellen, bei denen Sie kein neues Objekt zurückgeben möchten?
• Können Sie sich Komponenten vorstellen, die von der Schnittstelle abhängen (indem eine Instanz injiziert wird), für die der aktualisierte Mitarbeiter nicht als Rückgabewert benötigt wird?

Denken Sie auch an Scheinobjekte für Unit-Tests. Offensichtlich wird es einfacher sein, ohne den Rückgabewert zu verspotten. Und abhängige Komponenten lassen sich leichter testen, wenn ihre injizierten Abhängigkeiten einfachere Schnittstellen aufweisen.

Basierend auf diesen Überlegungen können Sie eine Entscheidung treffen.

Und wenn Sie es später bereuen, können Sie immer noch eine zweite Schnittstelle mit Adaptern zwischen den beiden einführen. Natürlich erhöhen solche zusätzlichen Schnittstellen die Komplexität der Komposition, so dass es ein Kompromiss ist.

Ihr Ansatz ist in Ordnung. Unveränderlichkeit ist eine starke Behauptung. Ich würde nur fragen: Gibt es einen anderen Ort, an dem Sie das Objekt konstruieren? Wenn Ihr Objekt nicht unveränderlich wäre, müssten Sie zusätzliche Fragen beantworten, da "State" eingeführt wird. Und die Zustandsänderung eines Objekts kann aus verschiedenen Gründen auftreten. Dann sollten Sie Ihre Fälle kennen und sie sollten nicht redundant oder geteilt sein.