FESTE vs. statische Methoden

Hier ist ein Problem, auf das ich häufig stoße: Es soll ein Webshop-Projekt mit einer Produktklasse geben. Ich möchte eine Funktion hinzufügen, mit der Benutzer Bewertungen zu einem Produkt veröffentlichen können. Ich habe also eine Review-Klasse, die auf ein Produkt verweist. Jetzt brauche ich eine Methode, die alle Bewertungen zu einem Produkt auflistet. Es gibt zwei Möglichkeiten:

(EIN)

public class Product {
  ...
  public Collection<Review> getReviews() {...}
}

(B)

public class Review {
  ...
  static public Collection<Review> forProduct( Product product ) {...}
}

Wenn ich mir den Code anschaue, würde ich (A) wählen: Er ist nicht statisch und benötigt keinen Parameter. Ich spüre jedoch, dass (A) das Prinzip der Einzelverantwortung (Single Responsibility Principle, SRP) und das Open-Closed-Prinzip (OCP) verletzt, während (B) nicht:

• (SRP) Wenn ich die Art und Weise ändern möchte, wie Bewertungen für ein Produkt gesammelt werden, muss ich die Produktklasse ändern. Es sollte jedoch nur einen Grund geben, die Produktklasse zu ändern. Und das sind sicherlich nicht die Bewertungen. Wenn ich alle Funktionen packe, die etwas mit Produkten in Product zu tun haben, wird es bald klappern.

• (OCP) Ich muss die Produktklasse ändern, um sie mit dieser Funktion zu erweitern. Ich denke, dies verstößt gegen den Teil des Prinzips, der für Veränderungen geschlossen ist. Bevor ich die Anfrage des Kunden zur Implementierung der Überprüfungen erhielt, betrachtete ich das Produkt als fertig und "schloss" es.

Was ist wichtiger: Befolgen Sie die SOLID-Prinzipien oder haben Sie eine einfachere Oberfläche?

Oder mache ich hier überhaupt etwas falsch?

Ergebnis

Wow, danke für all deine tollen Antworten! Es ist schwer, eine als offizielle Antwort auszuwählen.

Lassen Sie mich die Hauptargumente aus den Antworten zusammenfassen:

• Pro (A): OCP ist kein Gesetz und die Lesbarkeit des Codes ist ebenfalls wichtig.
• pro (A): Die Entitätsbeziehung sollte navigierbar sein. Beide Klassen kennen möglicherweise diese Beziehung.
• pro (A) + (B): Beides tun und in (A) an (B) delegieren, damit das Produkt weniger wahrscheinlich erneut geändert wird.
• pro (C): Finder-Methoden in die dritte Klasse (Service) einordnen, wo sie nicht statisch sind.
• contra (B): Verhindert das Verspotten in Tests.

Ein paar zusätzliche Dinge, die meine Hochschulen bei der Arbeit beigetragen haben:

• pro (B): Unser ORM-Framework kann automatisch den Code für (B) generieren.
• pro (A): Aus technischen Gründen unseres ORM-Frameworks muss in einigen Fällen die "geschlossene" Einheit geändert werden, unabhängig davon, wohin der Finder geht. Ich werde mich also sowieso nicht immer an SOLID halten können.
• contra (C): zu viel Aufhebens ;-)

Fazit

Ich verwende beide (A) + (B) mit Delegation für mein aktuelles Projekt. In einer serviceorientierten Umgebung werde ich jedoch mit (C) gehen.

Was ist wichtiger: Befolgen Sie die SOLID-Prinzipien oder haben Sie eine einfachere Oberfläche?

Schnittstelle gegenüber SOLID

Diese schließen sich nicht gegenseitig aus. Die Benutzeroberfläche sollte die Art Ihres Geschäftsmodells idealerweise in Bezug auf das Geschäftsmodell ausdrücken. Das SOLID-Prinzip ist ein Koan zur Maximierung der Wartbarkeit von objektorientiertem Code (und ich meine "Wartbarkeit" im weitesten Sinne). Ersteres unterstützt die Verwendung und Manipulation Ihres Geschäftsmodells und letzteres optimiert die Codepflege.

Offenes / geschlossenes Prinzip

"Fass das nicht an!" ist eine zu vereinfachende Interpretation. Und wenn wir "die Klasse" meinen, ist das willkürlich und nicht unbedingt richtig. OCP bedeutet vielmehr, dass Sie Ihren Code so gestaltet haben, dass Sie zum Ändern des Verhaltens nicht direkt den vorhandenen Arbeitscode ändern müssen (sollten). Darüber hinaus ist es ideal, den Code nicht zu berühren, um die Integrität vorhandener Schnittstellen zu erhalten. Dies ist meiner Ansicht nach eine bedeutende Folge von OCP.

Schließlich sehe ich OCP als Indikator für die vorhandene Designqualität. Wenn ich zu oft offene Klassen (oder Methoden) knacke und / oder ohne wirklich solide (ha, ha) Gründe dafür, kann dies bedeuten, dass ich ein schlechtes Design habe (und / oder nicht) weiß, wie man OO codiert).

Geben Sie Ihr Bestes, wir haben ein Team von Ärzten bereit

Wenn Ihre Anforderungsanalyse besagt, dass Sie die Produkt-Überprüfungs-Beziehung aus beiden Perspektiven ausdrücken müssen, tun Sie dies.

Deshalb, Wolfgang, haben Sie vielleicht einen guten Grund, diese vorhandenen Klassen zu ändern. Wenn angesichts der neuen Anforderungen eine Überprüfung nun ein grundlegender Bestandteil eines Produkts ist und jede Erweiterung des Produkts eine Überprüfung erfordert, wenn dies den Client-Code angemessen aussagekräftig macht, integrieren Sie ihn in das Produkt.

SOLID sind Richtlinien, also beeinflussen Sie die Entscheidungsfindung, anstatt sie zu diktieren.

Bei der Verwendung statischer Methoden ist zu beachten, dass sie sich auf die Testbarkeit auswirken .

Testen forProduct(Product product)ist kein Problem.

Etwas zu testen, das davon abhängt, wird sein.

Sie haben keine Naht zum Isolieren des abhängigen Code-Under-Test (CUT), um ein Modell zu verwenden, da bei Ausführung der Anwendung die statischen Methoden unbedingt vorhanden sein müssen.

Lassen Sie uns eine Methode namens CUT()aufrufenforProduct()

Wenn dies der Fall forProduct()ist static, können Sie nicht CUT()als atomare Einheit testen , und alle Ihre Tests werden zu Integrationstests, die die Logik der Testeinheit bestimmen.

Ein Fehler in einem Test für CUT kann durch ein Problem in CUT()oder in forProduct()(oder einem seiner abhängigen Codes) verursacht werden, das die Diagnosevorteile von Komponententests aufhebt.

Weitere Informationen finden Sie in diesem ausgezeichneten Blog-Beitrag: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html

Dies kann zu Frustrationen führen, wenn Tests nicht bestanden werden und bewährte Verfahren und Vorteile, die sie umgeben, aufgegeben werden.

Wenn Sie der Meinung sind, dass das Produkt der richtige Ort ist, um die Bewertungen des Produkts zu finden, können Sie dem Produkt jederzeit eine helfende Klasse geben, um die Arbeit für das Produkt zu erledigen. (Sie können feststellen, dass Ihr Unternehmen nur in Bezug auf ein Produkt über eine Bewertung spricht.)

Zum Beispiel wäre ich versucht, etwas zu injizieren, das die Rolle eines Review Retrievers spielt. Ich würde es wahrscheinlich die Schnittstelle geben IRetrieveReviews. Sie können dies in den Konstruktor des Produkts einfügen (Dependency Injection). Wenn Sie ändern möchten, wie die Bewertungen abgerufen werden, können Sie dies einfach tun, indem Sie einen anderen Mitarbeiter einsetzen - einen TwitterReviewRetrieveroder einen AmazonReviewRetrieveroder MultipleSourceReviewRetrieveroder was auch immer Sie sonst noch benötigen.

Beide haben jetzt eine einzige Verantwortung (sie sind die Anlaufstelle für alle produktbezogenen Dinge bzw. das Abrufen von Bewertungen), und in Zukunft kann das Verhalten des Produkts in Bezug auf Bewertungen geändert werden, ohne das Produkt tatsächlich zu ändern (Sie können es erweitern als ProductWithReviewsob Sie wirklich pedantisch über Ihre SOLID-Prinzipien sein wollten, aber das wäre gut genug für mich).

Ich hätte eine ProductsReview-Klasse. Sie sagen, Review ist sowieso neu. Das heißt nicht, dass es einfach alles sein kann. Es muss immer noch nur einen Grund geben, sich zu ändern. Wenn Sie ändern, wie Sie die Bewertungen aus irgendeinem Grund erhalten, müssen Sie die Überprüfungsklasse ändern.

Das ist nicht richtig

Sie fügen die statische Methode in die Review-Klasse ein, weil ... warum? Haben Sie nicht damit zu kämpfen? Ist das nicht das ganze Problem?

Dann nicht. Machen Sie eine Klasse, deren alleinige Verantwortung darin besteht, die Produktbewertungen zu erhalten. Sie können es dann in ProductReviewsByStartRating unterordnen. Oder unterklassifizieren Sie es, um Bewertungen für eine Produktklasse zu erhalten.

Ich würde die Funktion "Bewertungen für Produkte abrufen" weder in die ProductKlasse noch in die ReviewKlasse aufnehmen ...

Sie haben einen Ort, an dem Sie Ihre Produkte abrufen können, oder? Etwas mit GetProductById(int productId)und vielleicht GetProductsByCategory(int categoryId)und so weiter.

Ebenso sollten Sie einen Ort haben, an dem Sie Ihre Bewertungen abrufen können, mit einem GetReviewbyId(int reviewId)und vielleicht einem GetReviewsForProduct(int productId).

Wenn ich die Art und Weise ändern möchte, wie Bewertungen für ein Produkt gesammelt werden, muss ich die Produktklasse ändern.

Wenn Sie Ihre Daten Zugriff von Ihren Domain - Klassen trennen, werden Sie nicht ändern müssen entweder Domain - Klasse , wenn Sie die Art , wie die Bewertungen werden gesammelt ändern.

Muster und Prinzipien sind Richtlinien, keine in den Stein geschriebenen Regeln. Meiner Meinung nach ist die Frage nicht, ob es besser ist, den SOLID-Prinzipien zu folgen oder eine einfachere Oberfläche beizubehalten. Was Sie sich fragen sollten, ist, was für die meisten Menschen lesbarer und verständlicher ist. Oft bedeutet dies, dass es so nah wie möglich an der Domain sein muss.

In diesem Fall würde ich die Lösung (B) bevorzugen, da für mich der Ausgangspunkt das Produkt ist, nicht die Überprüfung, aber stellen Sie sich vor, Sie schreiben eine Software zum Verwalten von Bewertungen. In diesem Fall ist das Zentrum die Überprüfung, daher kann Lösung (A) vorzuziehen sein.

Wenn ich viele Methoden wie diese habe ("Verbindungen" zwischen Klassen), entferne ich sie alle außerhalb und erstelle eine (oder mehrere) neue statische Klassen, um sie zu organisieren. Normalerweise können Sie sie als Abfragen oder als eine Art Repository sehen.

Ihr Produkt kann nur an Ihre statische Überprüfungsmethode delegieren. In diesem Fall stellen Sie eine praktische Schnittstelle an einem natürlichen Ort bereit (Product.getReviews), Ihre Implementierungsdetails befinden sich jedoch in Review.getForProduct.

SOLID sind Richtlinien und sollten zu einer einfachen, sinnvollen Schnittstelle führen. Alternativ können Sie SOLID von einfachen, sinnvollen Schnittstellen ableiten. Es geht um das Abhängigkeitsmanagement innerhalb von Code. Ziel ist es, die Abhängigkeiten zu minimieren, die Reibung verursachen und Hindernisse für unvermeidliche Veränderungen schaffen.

Ich sehe das etwas anders als die meisten anderen Antworten. Ich denke das Productund Reviewsind im Grunde Datenübertragungsobjekte (DTOs). In meinem Code versuche ich, meine DTOs / Entitäten dazu zu bringen, Verhaltensweisen zu vermeiden. Sie sind nur eine nette API, um den aktuellen Status meines Modells zu speichern.

Wenn Sie über OO und SOLID sprechen, sprechen Sie im Allgemeinen von einem "Objekt", das nicht (notwendigerweise) den Status darstellt, sondern eine Art Dienst darstellt, der Fragen für Sie beantwortet oder an den Sie einen Teil Ihrer Arbeit delegieren können . Zum Beispiel:

interface IProductRepository
{
    void SaveNewProduct(IProduct product);
    IProduct GetProductById(ProductId productId);
    bool TryGetProductByName(string name, out IProduct product);
}

interface IProduct
{
    ProductId Id { get; }
    string Name { get; }
}

class ExistingProduct : IProduct
{
    public ProductId Id { get; private set; }
    public string Name { get; private set; }
}

Dann würde Ihr tatsächlicher ProductRepositoryein ExistingProductfür die GetProductByProductIdMethode usw. zurückgeben.

Jetzt folgen Sie dem Prinzip der Einzelverantwortung (was auch immer von erbt, IProducthält nur am Status fest, und was auch immer von erbt, IProductRepositoryist dafür verantwortlich, zu wissen, wie Sie Ihr Datenmodell beibehalten und rehydrieren können).

Wenn Sie Ihr Datenbankschema ändern, können Sie Ihre Repository-Implementierung ändern, ohne Ihre DTOs usw. zu ändern.

Kurz gesagt, ich denke, ich würde keine Ihrer Optionen wählen. :) :)

Statische Methoden sind möglicherweise schwieriger zu testen, aber das bedeutet nicht, dass Sie sie nicht verwenden können. Sie müssen sie nur so einrichten, dass Sie sie nicht testen müssen.

Machen Sie beide product.GetReviews und Review.ForProduct einzeilige Methoden, die so etwas wie sind

new ReviewService().GetReviews(productID);

ReviewService enthält den komplexeren Code und verfügt über eine Schnittstelle, die auf Testbarkeit ausgelegt ist, jedoch nicht direkt dem Benutzer zugänglich ist.

Wenn Sie eine 100% ige Abdeckung benötigen, lassen Sie Ihre Integrationstests die Methoden der Produkt- / Überprüfungsklasse aufrufen.

Es kann hilfreich sein, wenn Sie eher an das öffentliche API-Design als an das Klassendesign denken. In diesem Zusammenhang ist nur eine einfache Schnittstelle mit logischer Gruppierung von Bedeutung. Die tatsächliche Codestruktur ist nur für Entwickler von Bedeutung.