Verstößt dieses Klassendesign gegen das Prinzip der Einzelverantwortung?

Heute hatte ich einen Streit mit jemandem.

Ich habe die Vorteile eines Rich-Domain-Modells im Vergleich zu einem anämischen Domain-Modell erläutert. Und ich habe meinen Standpunkt mit einer einfachen Klasse demonstriert, die so aussieht:

public class Employee
{
    public Employee(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastname;
    }

    public string FirstName { get private set; }
    public string LastName { get; private set;}
    public int CountPaidDaysOffGranted { get; private set;}

    public void AddPaidDaysOffGranted(int numberOfdays)
    {
        // Do stuff
    }
}

Als er seinen anämischen Modellansatz verteidigte, lautete eines seiner Argumente: "Ich glaube an SOLID . Sie verletzen das Prinzip der einmaligen Verantwortung ( Single Responsibility Principle, SRP), da Sie sowohl Daten repräsentieren als auch Logik in derselben Klasse ausführen."

Ich fand diese Behauptung wirklich überraschend, da nach dieser Überlegung jede Klasse mit einer Eigenschaft und einer Methode die SRP verletzt und daher OOP im Allgemeinen nicht SOLID ist und funktionale Programmierung der einzige Weg zum Himmel ist.

Ich habe beschlossen, nicht auf seine vielen Argumente zu antworten, aber ich bin gespannt, was die Community zu dieser Frage denkt.

Wenn ich geantwortet hätte, hätte ich zunächst auf das oben erwähnte Paradox hingewiesen und dann darauf hingewiesen, dass die SRP in hohem Maße von der zu berücksichtigenden Granularität abhängt und dass, wenn Sie weit genug gehen, jede Klasse mehr als eine Klasse enthält Eigentum oder eine Methode verstößt dagegen.

Was hättest du gesagt?

Update: Das Beispiel wurde von guntbert großzügig aktualisiert, um die Methode realistischer zu gestalten und uns zu helfen, uns auf die zugrunde liegende Diskussion zu konzentrieren.

Die Einzelverantwortung ist als Abstraktion logischer Aufgaben in Ihrem System zu verstehen. Eine Klasse sollte die alleinige Verantwortung haben, (alles Notwendige zu tun, um) eine einzige spezifische Aufgabe auszuführen. Dies kann tatsächlich eine Menge in eine gut gestaltete Klasse bringen, abhängig von der Verantwortung. Die Klasse, in der Ihre Skript-Engine ausgeführt wird, kann beispielsweise eine Vielzahl von Methoden und Daten für die Verarbeitung von Skripten enthalten.

Ihr Mitarbeiter konzentriert sich auf das Falsche. Die Frage ist nicht "Welche Mitglieder hat diese Klasse?" aber "welche nützlichen Operationen führt diese Klasse innerhalb des Programms aus?" Sobald dies verstanden ist, sieht Ihr Domain-Modell gut aus.

Das Prinzip der Einzelverantwortung befasst sich nur mit der Frage, ob ein Teil des Codes (in OOP sprechen wir normalerweise von Klassen) für einen Teil der Funktionalität verantwortlich ist . Ich denke, Ihr Freund, der sagt, dass sich Funktionen und Daten nicht miteinander vermischen können, hat diese Idee nicht wirklich verstanden. Wenn Employeeer auch Informationen über seinen Arbeitsplatz, die Geschwindigkeit seines Autos und die Art des Futters, das sein Hund isst, enthalten würde, hätten wir ein Problem.

Da es in dieser Klasse nur um eine Employeegeht, ist es fair zu sagen, dass die SRP nicht offenkundig verletzt wird, aber die Leute werden immer ihre eigene Meinung haben.

Ein Ort, an dem wir uns verbessern könnten, ist die Trennung von Mitarbeiterinformationen (wie Name, Telefonnummer, E-Mail) von seinem Urlaub. Meiner Meinung nach bedeutet dies nicht, dass Methoden und Daten nicht miteinander vermischt werden können , sondern nur, dass sich die Urlaubsfunktionalität möglicherweise an einem anderen Ort befindet.

Meiner Meinung nach könnte diese Klasse möglicherweise SRP verletzen, wenn sie weiterhin sowohl ein Employeeals auch darstellt EmployeeHolidays.

So wie es ist, und wenn es für Peer Review zu mir käme, würde ich es wahrscheinlich durchlassen. Wenn mehr mitarbeiterspezifische Eigenschaften und Methoden hinzugefügt würden und mehr ferienspezifische Eigenschaften hinzugefügt würden, würde ich wahrscheinlich eine Aufteilung unter Berufung auf SRP und ISP empfehlen.

Es gibt bereits gute Antworten, die darauf hinweisen, dass SRP ein abstraktes Konzept für logische Funktionalität ist, aber es gibt subtilere Punkte, die meines Erachtens hinzugefügt werden sollten.

In den ersten beiden Buchstaben in SOLID, SRP und OCP wird beschrieben, wie sich Ihr Code als Reaktion auf eine Änderung der Anforderungen ändert. Meine Lieblingsdefinition der SRP lautet: "Ein Modul / eine Klasse / eine Funktion sollte nur einen Grund haben, sich zu ändern." Über wahrscheinliche Gründe für eine Änderung Ihres Codes zu streiten, ist viel produktiver als darüber zu streiten, ob Ihr Code SOLID ist oder nicht.

Wie viele Gründe muss Ihre Mitarbeiterklasse ändern? Ich weiß es nicht, weil ich den Kontext nicht kenne, in dem Sie es verwenden, und ich kann auch die Zukunft nicht sehen. Was ich tun kann, ist ein Brainstorming möglicher Änderungen basierend auf dem, was ich in der Vergangenheit gesehen habe, und Sie können subjektiv bewerten, wie wahrscheinlich sie sind. Wenn mehr als ein Punkt zwischen "einigermaßen wahrscheinlich" und "Mein Code hat sich genau aus diesem Grund bereits geändert" liegt, verstoßen Sie gegen die SRP gegen diese Art von Änderung. Hier ist eines: Jemand mit mehr als zwei Namen tritt Ihrem Unternehmen bei (oder ein Architekt liest diesen ausgezeichneten W3C-Artikel ). Und noch etwas: Ihr Unternehmen ändert die Zuordnung von Feiertagen.

Beachten Sie, dass diese Gründe auch dann gültig sind, wenn Sie die AddHolidays-Methode entfernen. Viele anämische Domänenmodelle verletzen die SRP. Viele von ihnen sind nur Datenbanktabellen im Code, und es kommt häufig vor, dass Datenbanktabellen mehr als 20 Änderungsgründe haben.

Hier ist etwas zu kauen: Würde sich Ihre Mitarbeiterklasse ändern, wenn Ihr System die Gehälter der Mitarbeiter nachverfolgen müsste? Adressen? Notfall-Kontaktinformationen? Wenn Sie zwei davon mit "Ja" (und "wahrscheinlich") bestätigen würden, würde Ihre Klasse die SRP verletzen, selbst wenn sie noch keinen Code enthält! Bei SOLID geht es sowohl um Prozesse und Architektur als auch um Code.

Dass die Klasse Daten darstellt, liegt nicht in der Verantwortung der Klasse, sondern in einem privaten Implementierungsdetail.

Die Klasse hat eine Verantwortung, einen Arbeitnehmer zu vertreten. In diesem Zusammenhang bedeutet dies, dass eine öffentliche API bereitgestellt wird, die Ihnen Funktionen bietet, die Sie für den Umgang mit Mitarbeitern benötigen (ob AddHolidays ein gutes Beispiel ist, ist umstritten).

Die Implementierung ist intern; es kommt also vor, dass dies einige private Variablen und eine Logik benötigt. Das bedeutet nicht, dass die Klasse jetzt mehrere Verantwortlichkeiten hat.

Die Idee, dass das Mischen von Logik und Daten in irgendeiner Weise immer falsch ist, ist so lächerlich, dass es nicht einmal einer Diskussion wert ist. In diesem Beispiel liegt zwar eine eindeutige Verletzung der Einzelverantwortung vor, aber nicht daran, dass es eine Eigenschaft DaysOfHolidaysund eine Funktion gibt AddHolidays(int).

Das liegt daran, dass sich die Identität des Mitarbeiters mit dem Urlaubsmanagement vermischt, was schlecht ist. Die Identität des Mitarbeiters ist eine komplexe Sache, die erforderlich ist, um Urlaub, Gehalt, Überstunden nachzuverfolgen, um darzustellen, wer zu welchem ​​Team gehört, um sich mit Leistungsberichten zu verknüpfen usw. Ein Mitarbeiter kann auch den Vor- und Nachnamen oder beides ändern und dabei gleich bleiben Mitarbeiter. Mitarbeiter können sogar mehrere Schreibweisen ihres Namens haben, z. B. eine ASCII- und eine Unicode-Schreibweise. Personen können 0 bis n Vor- und / oder Nachnamen haben. Sie können in verschiedenen Gerichtsbarkeiten unterschiedliche Namen haben. Das Nachverfolgen der Identität eines Mitarbeiters ist eine ausreichende Verantwortung, sodass das Ferien- oder Urlaubsmanagement nicht hinzugefügt werden kann, ohne es als zweite Verantwortung zu bezeichnen.

"Ich glaube an SOLID. Sie verletzen das Prinzip der einmaligen Verantwortung (Single Responsibility Principle, SRP), da Sie beide Daten repräsentieren und Logik in derselben Klasse ausführen."

Wie die anderen stimme ich dem nicht zu.

Ich würde sagen, dass die SRP verletzt wird, wenn Sie mehr als eine Logik in der Klasse ausführen . Wie viele Daten in der Klasse gespeichert werden müssen, um diese einzelne Logik zu erreichen, ist irrelevant.

Ich finde es heutzutage nicht sinnvoll, darüber zu diskutieren, was eine einzige Verantwortung oder einen einzigen Grund für eine Änderung darstellt oder nicht. Ich würde an seiner Stelle ein Minimum-Trauer-Prinzip vorschlagen:

Prinzip der minimalen Trauer: Code sollte entweder versuchen, die Wahrscheinlichkeit zu minimieren, dass Änderungen erforderlich sind, oder den Änderungskomfort maximieren.

Wie ist das? Ein Raketenwissenschaftler sollte nicht herausfinden, warum dies zur Senkung der Wartungskosten beitragen kann, und hoffentlich sollte es kein endloser Streitpunkt sein, aber wie bei SOLID im Allgemeinen ist es nicht überall blind anzuwenden. Es ist etwas zu beachten, während Kompromisse ausgeglichen werden.

Die Wahrscheinlichkeit, dass Änderungen erforderlich sind, hängt mit folgenden Faktoren zusammen:

1. Gute Prüfung (verbesserte Zuverlässigkeit).
2. Bezieht sich nur auf das Nötigste, um etwas Bestimmtes zu tun (dies kann das Reduzieren afferenter Kopplungen einschließen).
3. Machen Sie den Code einfach zu dem, was er tut (siehe Make Badass Principle).

Die Schwierigkeit, Änderungen vorzunehmen, geht mit efferenten Kupplungen einher. Das Testen führt zu efferenten Kupplungen, verbessert jedoch die Zuverlässigkeit. Gut gemacht, tut es im Allgemeinen mehr gut als schaden und ist absolut akzeptabel und wird durch das Minimum Grief-Prinzip gefördert.

Make Badass-Prinzip: Klassen, die an vielen Orten verwendet werden, sollten fantastisch sein. Sie sollten zuverlässig und effizient sein, wenn dies mit ihrer Qualität zusammenhängt usw.

Und das Make-Badass-Prinzip ist an das Minimum-Grief-Prinzip gebunden, da Badass-Dinge mit geringerer Wahrscheinlichkeit Änderungen erfordern als Dinge, die an dem scheißen, was sie tun.

Ich hätte zunächst auf das oben erwähnte Paradox hingewiesen und dann darauf hingewiesen, dass die SRP in hohem Maße von der zu berücksichtigenden Granularität abhängt und dass eine Klasse, die mehr als eine Eigenschaft oder eine Methode enthält, einen Verstoß darstellt, wenn Sie weit genug gehen es.

Aus SRP-Sicht hätte eine Klasse, die kaum etwas tut, sicherlich nur einen (manchmal null) Änderungsgrund:

class Float
{
public:
    explicit Float(float val);
    float get() const;
    void set(float new_val);
};

Das hat praktisch keinen Grund, sich zu ändern! Es ist besser als SRP. Es ist ZRP!

Außer ich würde vorschlagen, dass es eine offensichtliche Verletzung des Make Badass Prinzips ist. Es ist auch absolut wertlos. Etwas, das so wenig bewirkt, kann nicht hoffentlich schlecht sein. Es hat zu wenig Informationen (TLI). Und natürlich, wenn Sie etwas haben, das TLI ist, kann es nichts wirklich Bedeutendes bewirken, auch nicht mit den darin enthaltenen Informationen, und es muss nach außen geleitet werden, in der Hoffnung, dass jemand anderes tatsächlich etwas Bedeutendes und Böses tut. Und diese Undichtigkeit ist in Ordnung für etwas, das nur dazu gedacht ist, Daten und nichts weiter zu aggregieren, aber diese Schwelle ist der Unterschied, den ich zwischen "Daten" und "Objekten" sehe.

Natürlich ist etwas, was TMI ist, auch schlecht. Wir könnten unsere gesamte Software in eine Klasse einteilen. Es kann sogar nur eine runMethode geben. Und jemand könnte sogar argumentieren, dass es jetzt einen sehr allgemeinen Grund für eine Änderung gibt: "Diese Klasse muss nur geändert werden, wenn die Software verbessert werden muss." Ich bin dumm, aber natürlich können wir uns alle Wartungsprobleme damit vorstellen.

Es gibt also einen Spagat in Bezug auf die Granularität oder Grobheit der von Ihnen entworfenen Objekte. Ich beurteile es oft daran, wie viele Informationen Sie an die Außenwelt weitergeben müssen und wie viele sinnvolle Funktionen sie ausführen können. Ich finde das Make-Badass-Prinzip oft hilfreich, um das Gleichgewicht zu finden und es mit dem Minimum-Grief-Prinzip zu kombinieren.

Im Gegenteil, für mich bricht das anämische Domänenmodell einige OOP-Hauptkonzepte (die Attribute und Verhalten miteinander verbinden), kann jedoch aufgrund der Architekturentscheidungen unvermeidlich sein. Anämische Domänen sind einfacher zu denken, weniger organisch und sequentieller.

Viele Systeme tendieren dazu, dies zu tun, wenn mehrere Ebenen mit denselben Daten spielen müssen (Service-Ebene, Web-Ebene, Client-Ebene, Agenten ...).

Es ist einfacher, die Datenstruktur an einer Stelle und das Verhalten in anderen Serviceklassen zu definieren. Wenn dieselbe Klasse auf mehreren Ebenen verwendet wurde, wird diese möglicherweise größer und es stellt sich die Frage, welche Ebene für die Definition des erforderlichen Verhaltens verantwortlich ist und wer die Methoden aufrufen kann.

Dies ist möglicherweise keine gute Idee, als dass ein Agent-Prozess, der Statistiken aller Ihrer Mitarbeiter erstellt, eine Berechnung für bezahlte Tage aufruft. Und die GUI für die Mitarbeiterliste benötigt sicherlich überhaupt nicht die neue aggregierte ID-Berechnung, die in diesem Statistikagenten verwendet wird (und die technischen Daten, die damit einhergehen). Wenn Sie die Methoden auf diese Weise trennen, enden Sie im Allgemeinen mit einer Klasse, die nur Datenstrukturen enthält.

Sie können die "Objekt" -Daten oder nur einige von ihnen oder ein anderes Format (json) zu einfach serialisieren / deserialisieren, ohne sich um ein Objektkonzept / eine Objektverantwortung sorgen zu müssen. Es geht aber nur um Datenübermittlung. Sie können jederzeit Daten zwischen zwei Klassen zuordnen (Employee, EmployeeVO, EmployeeStatistic…), aber was bedeutet Employee hier wirklich?

Also ja, es trennt Daten in Domänenklassen und Datenbehandlung in Serviceklassen vollständig, aber es wird hier benötigt. Ein solches System ist gleichzeitig ein funktionales System, um geschäftlichen Wert zu schaffen, und ein technisches System, um die Daten überall dort zu verbreiten, wo sie benötigt werden, wobei ein angemessener Verantwortungsbereich eingehalten wird (und das verteilte Objekt löst dies auch nicht).

Wenn Sie keine separaten Verhaltensbereiche benötigen, können Sie die Methoden in Abhängigkeit davon, wie Sie Ihr Objekt sehen, in Dienstklassen oder in Domänenklassen einordnen. Ich neige dazu, ein Objekt als das "echte" Konzept zu sehen, dies hilft natürlich, SRP zu halten. In Ihrem Beispiel ist dies realistischer, als wenn der Chef des Mitarbeiters seinem PayDayAccount einen freien Zahltag hinzufügt. Ein Mitarbeiter ist von einer Firma eingestellt worden, Works kann krank sein oder um Rat gefragt werden, und er hat ein Zahltagkonto (der Chef kann es direkt bei ihm oder in einem PayDayAccount-Register abrufen ...). Sie können jedoch eine aggregierte Verknüpfung erstellen Hier der Einfachheit halber, wenn Sie für eine einfache Software nicht zu viel Komplexität benötigen.

Sie verstoßen gegen das Prinzip der einmaligen Verantwortung (SRP), da Sie sowohl Daten darstellen als auch Logik in derselben Klasse ausführen.

Das klingt für mich sehr vernünftig. Das Modell verfügt möglicherweise nicht über öffentliche Eigenschaften, wenn Aktionen verfügbar gemacht werden. Grundsätzlich handelt es sich um eine Idee zur Trennung von Befehlen und Abfragen. Bitte beachten Sie, dass Command mit Sicherheit einen privaten Status hat.

Sie können das Prinzip der Einzelverantwortung nicht verletzen, weil es nur ein ästhetisches Kriterium und keine Naturregel ist. Lassen Sie sich nicht durch den wissenschaftlich klingenden Namen und die Großbuchstaben irreführen.