Wie soll eine Mitarbeiterklasse gestaltet werden?

11

Ich versuche, ein Programm für die Verwaltung von Mitarbeitern zu erstellen. Ich kann jedoch nicht herausfinden, wie ich die EmployeeKlasse gestalten soll. Mein Ziel ist es, Mitarbeiterdaten in der Datenbank mithilfe eines EmployeeObjekts erstellen und bearbeiten zu können .

Die grundlegende Implementierung, an die ich dachte, war diese einfache:

class Employee
{
    // Employee data (let's say, dozens of properties).

    Employee() {}
    Create() {}
    Update() {}
    Delete() {}
}

Bei dieser Implementierung sind mehrere Probleme aufgetreten.

  1. Die IDein Mitarbeiter von der Datenbank gegeben wird, so dass , wenn ich das Objekt zur Beschreibung einen neuen Mitarbeiters verwende, wird es keine sein IDnoch zu speichern, während ein Objekt einen vorhandenen Mitarbeiter darstellen wird eine haben ID. Ich habe also eine Eigenschaft, die das Objekt manchmal beschreibt und manchmal nicht (Was könnte darauf hinweisen, dass wir gegen SRP verstoßen ? Da wir dieselbe Klasse für die Darstellung neuer und bestehender Mitarbeiter verwenden ...).
  2. Die CreateMethode soll einen Mitarbeiter in der Datenbank erstellen, während die Updateund Deleteauf einen vorhandenen Mitarbeiter einwirken sollen (erneut SRP ...).
  3. Welche Parameter sollte die Methode 'Erstellen' haben? Dutzende von Parametern für alle Mitarbeiterdaten oder vielleicht ein EmployeeObjekt?
  4. Sollte die Klasse unveränderlich sein?
  5. Wie wird die UpdateArbeit? Nimmt es die Eigenschaften und aktualisiert die Datenbank? Oder werden möglicherweise zwei Objekte - ein "altes" und ein "neues" - benötigt, um die Datenbank mit den Unterschieden zwischen ihnen zu aktualisieren? (Ich denke, die Antwort hat mit der Antwort über die Veränderlichkeit der Klasse zu tun).
  6. Was wäre die Verantwortung des Konstrukteurs? Was wären die Parameter, die es braucht? Würde es Mitarbeiterdaten mithilfe eines idParameters aus der Datenbank abrufen und diese mit den Eigenschaften füllen?

Wie Sie sehen, habe ich ein bisschen Unordnung im Kopf und bin sehr verwirrt. Könnten Sie mir bitte helfen zu verstehen, wie eine solche Klasse aussehen sollte?

Bitte beachten Sie, dass ich keine Meinungen haben möchte, nur um zu verstehen, wie eine so häufig verwendete Klasse im Allgemeinen gestaltet ist.

Sipo
quelle
3
Ihre derzeitige Verletzung des SRP besteht darin, dass Sie eine Klasse haben, die sowohl eine Entität darstellt als auch für die CRUD-Logik verantwortlich ist. Wenn Sie es trennen, dass CRUD-Operationen und die Entitätsstruktur unterschiedliche Klassen sind, brechen 1. und 2. SRP nicht. 3. Sollte ein EmployeeObjekt zur Abstraktion verwendet werden, sind die Fragen 4. und 5. im Allgemeinen nicht zu beantworten, hängen von Ihren Anforderungen ab. Wenn Sie die Struktur- und CRUD-Operationen in zwei Klassen unterteilen, ist es ziemlich klar, dass der Konstruktor von Employeekeine Daten abrufen kann von db mehr, so dass Antworten 6.
Andy
@ DavidPacker - Danke. Könnten Sie das in eine Antwort setzen?
Sipo
5
Ich wiederhole, lassen Sie Ihren Ctor nicht auf die Datenbank zugreifen. Wenn Sie dies tun, wird der Code eng mit der Datenbank verbunden, und es ist schrecklich schwierig, Dinge zu testen (selbst manuelle Tests werden schwieriger). Schauen Sie sich das Repository-Muster an. Denken Sie eine Sekunde darüber nach, sind Sie Updateein Mitarbeiter oder aktualisieren Sie einen Mitarbeiterdatensatz? Hast du Employee.Delete()oder macht ein Boss.Fire(employee)?
RubberDuck
1
Ist es für Sie, abgesehen von dem, was bereits erwähnt wurde, sinnvoll, dass Sie einen Mitarbeiter benötigen, um einen Mitarbeiter zu erstellen? Im aktiven Datensatz ist es möglicherweise sinnvoller, einen Mitarbeiter neu zu erstellen und dann für dieses Objekt Speichern aufzurufen. Selbst dann haben Sie jetzt eine Klasse, die sowohl für die Geschäftslogik als auch für die eigene Datenpersistenz verantwortlich ist.
Herr Cochese

Antworten:

10

Dies ist eine wohlgeformtere Transkription meines ersten Kommentars unter Ihrer Frage. Die Antworten auf Fragen, die vom OP beantwortet werden, finden Sie am Ende dieser Antwort. Bitte überprüfen Sie auch den wichtigen Hinweis am selben Ort.


Was Sie derzeit beschreiben, Sipo, ist ein Entwurfsmuster namens Active Record . Wie bei allem hat auch dieser seinen Platz unter den Programmierern gefunden, wurde jedoch aus einem einfachen Grund, der Skalierbarkeit, zugunsten des Repositorys und der Data Mapper- Muster verworfen .

Kurz gesagt, ein aktiver Datensatz ist ein Objekt, das:

  • stellt ein Objekt in Ihrer Domäne dar (enthält Geschäftsregeln, weiß, wie bestimmte Vorgänge für das Objekt ausgeführt werden, z. B. ob Sie einen Benutzernamen ändern können oder nicht usw.),
  • weiß, wie die Entität abgerufen, aktualisiert, gespeichert und gelöscht wird.

Sie sprechen mehrere Probleme mit Ihrem aktuellen Design an, und das Hauptproblem Ihres Designs wird im letzten, sechsten Punkt angesprochen (last but not least, denke ich). Wenn Sie eine Klasse haben, für die Sie einen Konstruktor entwerfen, und Sie nicht einmal wissen, was der Konstruktor tun soll, macht die Klasse wahrscheinlich etwas falsch. Das ist in deinem Fall passiert.

Das Korrigieren des Entwurfs ist jedoch ziemlich einfach, indem die Entitätsdarstellung und die CRUD-Logik in zwei (oder mehr) Klassen aufgeteilt werden.

So sieht Ihr Design jetzt aus:

  • Employee- enthält Informationen über die Mitarbeiterstruktur (ihre Attribute) und Methoden zum Ändern der Entität (wenn Sie sich für einen veränderlichen Weg entscheiden), enthält CRUD-Logik für die EmployeeEntität, kann eine Liste von EmployeeObjekten zurückgeben und akzeptiert ein EmployeeObjekt, wenn Sie möchten Aktualisieren Sie einen Mitarbeiter, kann eine einzelne Employeeüber eine Methode wie zurückgebengetSingleById(id : string) : Employee

Wow, die Klasse scheint riesig.

Dies wird die vorgeschlagene Lösung sein:

  • Employee - enthält Informationen über die Mitarbeiterstruktur (ihre Attribute) und Methoden zum Ändern der Entität (wenn Sie sich für einen veränderlichen Weg entscheiden)
  • EmployeeRepository- enthält CRUD-Logik für die EmployeeEntität, kann eine Liste von EmployeeObjekten zurückgeben, akzeptiert ein EmployeeObjekt, wenn Sie einen Mitarbeiter aktualisieren möchten, kann ein einzelnes Employeeüber eine Methode wie zurückgebengetSingleById(id : string) : Employee

Haben Sie von der Trennung von Bedenken gehört ? Nein, das wirst du jetzt. Es ist die weniger strenge Version des Prinzips der Einzelverantwortung, die besagt, dass eine Klasse eigentlich nur eine Verantwortung haben sollte, oder wie Onkel Bob sagt:

Ein Modul sollte nur einen Grund zur Änderung haben.

Es ist ziemlich klar, dass, wenn ich Ihre anfängliche Klasse klar in zwei Klassen aufteilen konnte, die immer noch eine gut abgerundete Oberfläche haben, die anfängliche Klasse wahrscheinlich zu viel getan hat, und das war es auch.

Das Besondere am Repository-Muster ist, dass es nicht nur als Abstraktion dient, um eine mittlere Schicht zwischen der Datenbank (die alles sein kann, Datei, NoSQL, SQL, objektorientiert) bereitzustellen, sondern auch nicht konkret sein muss Klasse. In vielen OO-Sprachen können Sie die Schnittstelle als eine tatsächliche interface(oder eine Klasse mit einer rein virtuellen Methode, wenn Sie sich in C ++ befinden) definieren und dann mehrere Implementierungen durchführen.

Dies hebt die Entscheidung, ob ein Repository eine tatsächliche Implementierung von Ihnen ist, vollständig auf. Sie verlassen sich einfach auf die Schnittstelle, indem Sie sich tatsächlich auf eine Struktur mit dem interfaceSchlüsselwort verlassen. Und Repository ist genau das. Es ist ein ausgefallener Begriff für die Abstraktion von Datenschichten, nämlich das Zuordnen von Daten zu Ihrer Domain und umgekehrt.

Eine weitere großartige Sache bei der Aufteilung in (mindestens) zwei Klassen ist, dass die EmployeeKlasse jetzt ihre eigenen Daten klar verwalten und es sehr gut machen kann, da sie sich nicht um andere schwierige Dinge kümmern muss.

Frage 6: Was soll der Konstruktor in der neu erstellten EmployeeKlasse tun ? Es ist einfach. Es sollte die Argumente übernehmen, prüfen, ob sie gültig sind (z. B. sollte ein Alter wahrscheinlich nicht negativ sein oder der Name sollte nicht leer sein), einen Fehler auslösen, wenn die Daten ungültig waren und wenn die bestandene Validierung die Argumente privaten Variablen zuweist des Unternehmens. Es kann jetzt nicht mit der Datenbank kommunizieren, da es einfach keine Ahnung hat, wie es geht.


Frage 4: Kann überhaupt nicht beantwortet werden, nicht generell, da die Antwort stark davon abhängt, was genau Sie brauchen.


Frage 5: Nun , da Sie die aufgeblähte Klasse in zwei getrennt haben, können Sie auf der mehrere Update - Methoden direkt haben EmployeeKlasse, wie changeUsername, markAsDeceased, die die Daten der Manipulation EmployeeKlasse nur im RAM und dann könnte man ein Verfahren , wie zum Beispiel vorstellt registerDirtyvon der Arbeitseinheitsmuster für die Repository-Klasse, über das Sie dem Repository mitteilen würden, dass dieses Objekt Eigenschaften geändert hat und nach dem Aufrufen der commitMethode aktualisiert werden muss .

Offensichtlich muss ein Objekt für ein Update eine ID haben und daher bereits gespeichert sein. Es ist die Verantwortung des Repositorys, dies zu erkennen und einen Fehler auszulösen, wenn die Kriterien nicht erfüllt sind.


Frage 3: Wenn Sie sich für das Arbeitseinheitsmuster entscheiden, lautet die createMethode jetzt registerNew. Wenn Sie dies nicht tun, würde ich es wahrscheinlich savestattdessen nennen. Das Ziel eines Repositorys ist es, eine Abstraktion zwischen der Domäne und der Datenschicht bereitzustellen. Aus diesem Grund würde ich Ihnen empfehlen, dass diese Methode (sei es registerNewoder save) das EmployeeObjekt akzeptiert und es den Klassen überlassen bleibt, die die Repository-Schnittstelle implementieren, welche Attribute Sie beschließen, das Unternehmen zu verlassen. Das Übergeben eines gesamten Objekts ist besser, sodass Sie nicht viele optionale Parameter benötigen.


Frage 2: Beide Methoden werden nun Teil der Repository-Schnittstelle und verstoßen nicht gegen das Prinzip der Einzelverantwortung. Die Verantwortung des Repositorys besteht darin, CRUD-Operationen für die EmployeeObjekte bereitzustellen. Dies ist die Aufgabe (neben Lesen und Löschen übersetzt CRUD sowohl Erstellen als auch Aktualisieren). Natürlich könnten Sie das Repository noch weiter aufteilen, indem Sie ein EmployeeUpdateRepositoryund so weiter haben, aber das wird selten benötigt und eine einzelne Implementierung kann normalerweise alle CRUD-Operationen enthalten.


Frage 1: Sie haben eine einfache EmployeeKlasse erhalten, die jetzt (unter anderem) die ID hat. Ob die ID gefüllt oder leer (oder null) ist, hängt davon ab, ob das Objekt bereits gespeichert wurde. Trotzdem ist eine ID immer noch ein Attribut, das die Entität besitzt, und die Verantwortung der EmployeeEntität besteht darin, sich um ihre Attribute und damit um ihre ID zu kümmern.

Ob eine Entität eine ID hat oder nicht, spielt normalerweise keine Rolle, bis Sie versuchen, eine Persistenzlogik darauf zu erstellen. Wie in der Antwort auf Frage 5 erwähnt, liegt es in der Verantwortung des Repositorys, festzustellen, dass Sie nicht versuchen, eine bereits gespeicherte Entität zu speichern oder eine Entität ohne ID zu aktualisieren.


Wichtige Notiz

Bitte beachten Sie, dass das Entwerfen einer funktionalen Repository-Schicht zwar eine große Trennung der Bedenken ist, aber eine ziemlich mühsame Arbeit ist und meiner Erfahrung nach etwas schwieriger zu erreichen ist als der Ansatz der aktiven Aufzeichnung. Am Ende erhalten Sie jedoch ein Design, das weitaus flexibler und skalierbarer ist, was eine gute Sache sein kann.

Andy
quelle
Hmm wie meine Antwort, aber nicht wie 'edgey' Schatten
Ewan
2
@Ewan Ich habe Ihre Antwort nicht abgelehnt, aber ich kann sehen, warum einige haben. Einige der Fragen des OP werden nicht direkt beantwortet, und einige Ihrer Vorschläge scheinen unbegründet zu sein.
Andy
1
Schöne und umfassende Antwort. Trifft den Nagel auf den Kopf mit der Trennung der Besorgnis. Und ich mag die Warnung, die die wichtige Wahl zwischen einem perfekten komplexen Design und einem schönen Kompromiss darstellt.
Christophe
Richtig, Ihre Antwort ist überlegen
Ewan
Wenn Sie zum ersten Mal ein neues Mitarbeiterobjekt erstellen, hat die ID keinen Wert. Das ID-Feld kann mit dem Wert Null verlassen werden, führt jedoch dazu, dass sich das Mitarbeiterobjekt im ungültigen Zustand befindet.
Susantha7
2

Erstellen Sie zunächst eine Mitarbeiterstruktur mit den Eigenschaften des konzeptionellen Mitarbeiters.

Erstellen Sie dann eine Datenbank mit einer passenden Tabellenstruktur, z. B. mssql

Erstellen Sie dann ein Mitarbeiter-Repository für diese Datenbank EmployeeRepoMsSql mit den verschiedenen CRUD-Operationen, die Sie benötigen.

Erstellen Sie dann eine IEmployeeRepo-Schnittstelle, die die CRUD-Operationen verfügbar macht

Erweitern Sie dann Ihre Employee-Struktur zu einer Klasse mit dem Konstruktionsparameter IEmployeeRepo. Fügen Sie die verschiedenen erforderlichen Methoden zum Speichern / Löschen usw. hinzu und implementieren Sie sie mit dem injizierten EmployeeRepo.

Wenn es sich um Id handelt, schlage ich vor, dass Sie eine GUID verwenden, die über Code im Konstruktor generiert werden kann.

Um mit vorhandenen Objekten zu arbeiten, kann Ihr Code diese über das Repository aus der Datenbank abrufen, bevor Sie deren Aktualisierungsmethode aufrufen.

Alternativ können Sie sich für das verpönte (aber meiner Ansicht nach überlegene) Anemic Domain Object-Modell entscheiden, bei dem Sie Ihrem Objekt keine CRUD-Methoden hinzufügen und das Objekt einfach an das Repo übergeben, um es zu aktualisieren / speichern / löschen

Unveränderlichkeit ist eine Designentscheidung, die von Ihren Mustern und Ihrem Codierungsstil abhängt. Wenn Sie alle funktionsfähig sind, versuchen Sie auch, unveränderlich zu sein. Wenn Sie sich nicht sicher sind, ist ein veränderliches Objekt wahrscheinlich einfacher zu implementieren.

Anstelle von Create () würde ich mit Save () gehen. Erstellen funktioniert mit dem Unveränderlichkeitskonzept, aber ich finde es immer nützlich, ein Objekt erstellen zu können, das noch nicht "gespeichert" ist, z. B. wenn Sie eine Benutzeroberfläche haben, mit der Sie ein oder mehrere Mitarbeiterobjekte auffüllen und sie dann vor einigen Regeln erneut überprüfen können Speichern in der Datenbank.

***** Beispielcode

public class Employee
{
    public string Id { get; set; }

    public string Name { get; set; }

    private IEmployeeRepo repo;

    //with the OOP approach you want the save method to be on the Employee Object
    //so you inject the IEmployeeRepo in the Employee constructor
    public Employee(IEmployeeRepo repo)
    {
        this.repo = repo;
        this.Id = Guid.NewGuid().ToString();
    }

    public bool Save()
    {
        return repo.Save(this);
    }
}

public interface IEmployeeRepo
{
    bool Save(Employee employee);

    Employee Get(string employeeId);
}

public class EmployeeRepoSql : IEmployeeRepo
{
    public Employee Get(string employeeId)
    {
        var sql = "Select * from Employee where Id=@Id";
        //more db code goes here
        Employee employee = new Employee(this);
        //populate object from datareader
        employee.Id = datareader["Id"].ToString();

    }

    public bool Save(Employee employee)
    {
        var sql = "Insert into Employee (....";
        //db logic
    }
}

public class MyADMProgram
{
    public void Main(string id)
    {
        //with ADM don't inject the repo into employee, just use it in your program
        IEmployeeRepo repo = new EmployeeRepoSql();
        var emp = repo.Get(id);

        //do business logic
        emp.Name = TextBoxNewName.Text;

        //save to DB
        repo.Save(emp);

    }
}
Ewan
quelle
1
Das anämische Domänenmodell hat sehr wenig mit der CRUD-Logik zu tun. Es ist ein Modell, das zwar zur Domänenschicht gehört, jedoch keine Funktionalität aufweist und alle Funktionen über Dienste bereitgestellt werden, an die dieses Domänenmodell als Parameter übergeben wird.
Andy
Genau in diesem Fall ist das Repo der Service und die Funktionen sind die CRUD-Operationen.
Ewan
@ DavidPacker sagen Sie, dass das anämische Domänenmodell eine gute Sache ist?
candied_orange
1
@CandiedOrange Ich habe meine Meinung in dem Kommentar nicht geäußert, aber nein, wenn Sie sich dazu entschließen, Ihre Anwendung auf Ebenen zu verteilen, auf denen eine Ebene nur für die Geschäftslogik verantwortlich ist, bin ich mit Mr. Fowler ein anämisches Domänenmodell ist in der Tat ein Anti-Muster. Warum sollte ich einen UserUpdateService mit einer changeUsername(User user, string newUsername)Methode benötigen , wenn ich die changeUsernameMethode genauso gut Userdirekt zur Klasse hinzufügen kann? Einen Service dafür zu erstellen ist unsinnig.
Andy
1
Ich denke in diesem Fall ist es nicht optimal, das Repo zu injizieren, nur um die CRUD-Logik auf das Modell zu setzen.
Ewan
1

Überprüfung Ihres Designs

Sie Employeesind in Wirklichkeit eine Art Proxy für ein Objekt, das dauerhaft in der Datenbank verwaltet wird.

Ich schlage daher vor, an die ID zu denken, als wäre sie ein Verweis auf Ihr Datenbankobjekt. Unter Berücksichtigung dieser Logik können Sie Ihr Design wie bei Nicht-Datenbankobjekten fortsetzen. Mit der ID können Sie die traditionelle Kompositionslogik implementieren:

  • Wenn die ID festgelegt ist, haben Sie ein entsprechendes Datenbankobjekt.
  • Wenn die ID nicht festgelegt ist, gibt es kein entsprechendes Datenbankobjekt: EmployeeMöglicherweise muss es noch erstellt werden, oder es wurde einfach gelöscht.
  • Sie benötigen einen Mechanismus, um die Beziehung für vorhandene Mitarbeiter und vorhandene Datenbankdatensätze zu initiieren, die noch nicht im Speicher geladen sind.

Sie müssten auch einen Status für das Objekt verwalten. Beispielsweise:

  • Wenn ein Mitarbeiter noch nicht über das Erstellen oder Abrufen von Daten mit einem DB-Objekt verknüpft ist, sollten Sie keine Aktualisierungen oder Löschungen durchführen können
  • Sind die Mitarbeiterdaten im Objekt mit der Datenbank synchronisiert oder wurden Änderungen vorgenommen?

Vor diesem Hintergrund könnten wir uns entscheiden für:

class Employee
{
    ...
    Employee () {}       // Initialize an empty Employee
    Load(IDType ID) {}   // Load employee with known ID from the database
    bool Create() {}     // Create an new employee an set its ID 
    bool Update() {}     // Update the employee (can ID be changed?)
    bool Delete() {}     // Delete the employee (and reset ID because there's no corresponding ID. 
    bool isClean () {}   // true if ID empty or if all properties match database
}

Um Ihren Objektstatus zuverlässig verwalten zu können, müssen Sie eine bessere Kapselung sicherstellen, indem Sie die Eigenschaften privat machen und den Zugriff nur über Getter und Setter gewähren, die den Status der Setter aktualisieren.

Deine Fragen

  1. Ich denke, die ID-Eigenschaft verletzt nicht die SRP. Die einzige Verantwortung besteht darin, auf ein Datenbankobjekt zu verweisen.

  2. Ihr Mitarbeiter als Ganzes ist nicht mit dem SRP kompatibel, da er für die Verknüpfung mit der Datenbank verantwortlich ist, aber auch für das Speichern temporärer Änderungen und für alle Transaktionen, die mit diesem Objekt ausgeführt werden.

    Ein anderes Design könnte darin bestehen, die veränderbaren Felder in einem anderen Objekt zu belassen, das nur geladen wird, wenn auf Felder zugegriffen werden muss.

    Sie können die Datenbanktransaktionen auf dem Mitarbeiter mithilfe des Befehlsmusters implementieren . Diese Art von Design würde auch die Entkopplung zwischen Ihren Geschäftsobjekten (Mitarbeiter) und Ihrem zugrunde liegenden Datenbanksystem erleichtern, indem datenbankspezifische Redewendungen und APIs isoliert werden.

  3. Ich würde nicht ein Dutzend Parameter hinzufügen Create(), da sich die Geschäftsobjekte weiterentwickeln und die Wartung sehr schwierig machen könnten. Und der Code würde unlesbar werden. Hier haben Sie zwei Möglichkeiten: Entweder übergeben Sie einen minimalistischen Satz von Parametern (nicht mehr als 4), die für die Erstellung eines Mitarbeiters in der Datenbank unbedingt erforderlich sind, und führen die verbleibenden Änderungen per Update durch, oder Sie übergeben ein Objekt. Übrigens verstehe ich in Ihrem Design, dass Sie bereits gewählt haben : my_employee.Create().

  4. Sollte die Klasse unveränderlich sein? Siehe obige Diskussion: In Ihrem Originalentwurf Nr. Ich würde mich für einen unveränderlichen Ausweis entscheiden, aber nicht für einen unveränderlichen Mitarbeiter. Ein Mitarbeiter entwickelt sich im wirklichen Leben (neue Arbeitsstelle, neue Adresse, neue eheliche Situation, sogar neue Namen ...). Ich denke, es wird einfacher und natürlicher sein, mit dieser Realität zu arbeiten, zumindest in der Ebene der Geschäftslogik.

  5. Wenn Sie einen Befehl zum Aktualisieren und ein bestimmtes Objekt für (GUI?) Verwenden möchten, um die gewünschten Änderungen zu speichern, können Sie sich für einen alten / neuen Ansatz entscheiden. In allen anderen Fällen würde ich ein veränderbares Objekt aktualisieren. Achtung: Das Update kann Datenbankcode auslösen, sodass Sie nach einem Update sicherstellen sollten, dass das Objekt immer noch wirklich mit der Datenbank synchronisiert ist.

  6. Ich denke, dass das Abrufen eines Mitarbeiters aus der Datenbank im Konstruktor keine gute Idee ist, da das Abrufen schief gehen kann und es in vielen Sprachen schwierig ist, mit fehlgeschlagenen Konstruktionen umzugehen. Der Konstruktor sollte das Objekt (insbesondere die ID) und seinen Status initialisieren.

Christophe
quelle