SOLID vs méthodes statiques

Voici un problème que je rencontre fréquemment: qu'il y ait un projet de boutique en ligne ayant une classe de produit. Je souhaite ajouter une fonctionnalité qui permet aux utilisateurs de publier des avis sur un produit. J'ai donc une classe Review qui fait référence à un produit. Maintenant, j'ai besoin d'une méthode qui répertorie toutes les critiques d'un produit. Il y a deux possibilités:

(UNE)

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

(B)

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

En regardant le code, je choisirais (A): Ce n'est pas statique et il n'a pas besoin de paramètre. Cependant, je sens que (A) viole le principe de responsabilité unique (SRP) et le principe ouvert-fermé (OCP) tandis que (B) ne:

• (SRP) Lorsque je veux changer la façon dont les avis sont collectés pour un produit, je dois changer la classe de produit. Mais il ne devrait y avoir qu'une seule raison pour changer la classe de produit. Et ce n'est certainement pas les critiques. Si j'emballe toutes les fonctionnalités qui ont quelque chose à voir avec les produits dans Product, elles seront bientôt claquées.

• (OCP) Je dois changer la classe de produit pour l'étendre avec cette fonctionnalité. Je pense que cela viole la partie «Fermé pour le changement» du principe. Avant de recevoir la demande du client pour la mise en œuvre des avis, j'ai considéré le produit comme terminé et je l'ai "fermé".

Quoi de plus important: suivre les principes SOLID, ou avoir une interface plus simple?

Ou est-ce que je fais quelque chose de mal ici?

Résultat

Wow, merci pour toutes vos bonnes réponses! Il est difficile d'en choisir une comme réponse officielle.

Permettez-moi de résumer les principaux arguments des réponses:

• pro (A): OCP n'est pas une loi et la lisibilité du code est également importante.
• pro (A): la relation d'entité doit être navigable. Les deux classes peuvent connaître cette relation.
• pro (A) + (B): faites les deux et déléguez (A) à (B) afin que le produit soit moins susceptible d'être modifié à nouveau.
• pro (C): mettez les méthodes de recherche dans la troisième classe (service) où elle n'est pas statique.
• contra (B): empêche la moquerie dans les tests.

Quelques choses supplémentaires que mes collèges au travail ont apportées:

• pro (B): notre framework ORM peut générer automatiquement le code pour (B).
• pro (A): pour des raisons techniques de notre framework ORM, il faudra dans certains cas modifier l'entité "fermée", indépendamment de l'endroit où se trouve le chercheur. Je ne pourrai donc pas toujours m'en tenir à SOLID de toute façon.
• contra (C): trop de bruit ;-)

Conclusion

J'utilise à la fois (A) + (B) avec délégation pour mon projet actuel. Dans un environnement axé sur les services, cependant, j'irai avec (C).

Quoi de plus important: suivre les principes SOLID, ou avoir une interface plus simple?

Interface face à SOLID

Ceux-ci ne sont pas mutuellement exclusifs. L'interface doit exprimer la nature de votre modèle d'entreprise idéalement en termes de modèle d'entreprise. Les principes SOLID sont un Koan pour maximiser la maintenabilité du code orienté objet (et je veux dire "maintenabilité" au sens le plus large). Le premier prend en charge l'utilisation et la manipulation de votre modèle d'entreprise et le second optimise la maintenance du code.

Principe ouvert / fermé

"ne touche pas à ça!" est une interprétation trop simpliste. Et en supposant que nous voulons dire «la classe» est arbitraire et pas nécessairement juste. OCP signifie plutôt que vous avez conçu votre code de telle sorte que la modification de son comportement ne vous oblige pas (ne devrait pas) modifier directement le code existant. En outre, ne pas toucher au code en premier lieu est le moyen idéal pour préserver l'intégrité des interfaces existantes; c'est un corollaire significatif de l'OCP à mon avis.

Enfin, je vois OCP comme un indicateur de la qualité de conception existante. Si je me retrouve trop souvent à casser des classes ouvertes (ou des méthodes) et / ou sans des raisons vraiment solides (ha, ha) de le faire, cela peut me dire que j'ai une mauvaise conception (et / ou je ne le fais pas) savoir coder OO).

Donnez-vous votre meilleur coup, nous avons une équipe de médecins à vos côtés

Si votre analyse des exigences vous indique que vous devez exprimer la relation Product-Review des deux points de vue, faites-le.

Par conséquent, Wolfgang, vous pouvez avoir une bonne raison de modifier ces classes existantes. Compte tenu des nouvelles exigences, si une révision est désormais une partie fondamentale d'un produit, si chaque extension du produit doit être revue, si cela rend le code client correctement expressif, intégrez-le dans le produit.

SOLIDES sont des lignes directrices, donc influencez la prise de décision plutôt que de la dicter.

Une chose à savoir lors de l'utilisation de méthodes statiques est leur impact sur la testabilité .

Les tests forProduct(Product product)ne seront pas un problème.

Tester quelque chose qui en dépendra sera.

Vous n'aurez pas de couture pour isoler le code sous test (CUT) dépendant pour utiliser une maquette, car lorsque l'application est en cours d'exécution, les méthodes statiques existent nécessairement.

Ayons une méthode appelée CUT()qui appelleforProduct()

Si forProduct()c'est le cas, staticvous ne pouvez pas tester en CUT()tant qu'unité atomique et tous vos tests deviennent des tests d'intégration qui testent la logique de l'unité.

Un échec dans un test de CUT pourrait être causé par un problème dans CUT()ou dans forProduct()(ou l'un de ses codes dépendants) qui supprime les avantages de diagnostic des tests unitaires.

Voir cet excellent article de blog pour des informations plus détaillées: http://googletesting.blogspot.com/2008/12/static-methods-are-death-to-testability.html

Cela peut conduire à la frustration de l'échec des tests et à l'abandon des bonnes pratiques et des avantages qui les entourent.

Si vous pensez que le produit est le bon endroit pour trouver les avis sur le produit, vous pouvez toujours donner au produit une classe d'aide pour faire le travail pour lui. (Vous pouvez le dire parce que votre entreprise ne parlera jamais d'un avis, sauf en termes de produit).

Par exemple, je serais tenté d'injecter quelque chose qui a joué le rôle d'un retriever de révision. Je lui donnerais probablement l'interface IRetrieveReviews. Vous pouvez le mettre dans le constructeur du produit (injection de dépendance). Si vous souhaitez modifier la façon dont les avis sont récupérés, vous pouvez le faire facilement en injectant un collaborateur différent - un TwitterReviewRetrieverou un AmazonReviewRetrieverou MultipleSourceReviewRetrieverou tout ce dont vous avez besoin.

Les deux ont désormais une seule responsabilité (être la référence pour tout ce qui concerne le produit et récupérer les avis, respectivement), et à l'avenir le comportement du produit par rapport aux avis peut être modifié sans réellement changer le produit (vous pouvez l'étendre comme ProductWithReviewssi vous vouliez vraiment être pédant sur vos principes SOLIDES, mais ce serait assez bon pour moi).

J'aurais une classe ProductsReview. Vous dites que l'examen est nouveau de toute façon. Cela ne veut pas dire que ça peut être n'importe quoi. Il ne doit encore avoir qu'une seule raison de changer. Si vous modifiez la façon dont vous obtenez les avis pour une raison quelconque, vous devrez changer la classe d'avis.

Ce n'est pas correct.

Vous mettez la méthode statique dans la classe Review parce que ... pourquoi? N'est-ce pas ce avec quoi vous luttez? N'est-ce pas là tout le problème?

Alors ne le fais pas. Faites une classe dont la seule responsabilité est d'obtenir les avis sur les produits. Vous pouvez ensuite le sous-classer dans ProductReviewsByStartRating. Ou sous-classe pour obtenir des avis sur une classe de produits.

Je ne mettrais pas la fonctionnalité "Get Reviews For Product" dans la Productclasse ni dans la Reviewclasse ...

Vous avez un endroit où récupérer vos produits, non? Quelque chose avec GetProductById(int productId)et peut-être GetProductsByCategory(int categoryId)et ainsi de suite.

De même, vous devriez avoir un endroit pour récupérer vos avis, avec un GetReviewbyId(int reviewId)et peut-être un GetReviewsForProduct(int productId).

Lorsque je souhaite modifier la façon dont les avis sont collectés pour un produit, je dois changer la classe de produit.

Si vous séparez votre accès aux données de vos classes de domaine, vous n'aurez pas besoin de modifier l'une ou l' autre classe de domaine lorsque vous modifiez la façon dont les avis sont collectés.

Les modèles et les principes sont des lignes directrices, pas des règles écrites dans la pierre. À mon avis, la question n'est pas de savoir s'il vaut mieux suivre les principes SOLIDES ou garder une interface plus simple. Ce que vous devez vous demander, c'est ce qui est plus lisible et compréhensible pour la plupart des gens. Cela signifie souvent qu'il doit être aussi proche que possible du domaine.

Dans ce cas, je préférerais la solution (B) car pour moi, le point de départ est le produit, pas la critique, mais imaginez que vous écrivez un logiciel pour gérer les critiques. Dans ce cas, le centre est la revue, la solution (A) peut donc être préférable.

Lorsque j'ai beaucoup de méthodes comme celle-ci ("connexions" entre les classes), je les supprime toutes à l'extérieur et je crée une (ou plusieurs) nouvelle classe statique pour les organiser. Habituellement, vous pouvez les voir sous forme de requêtes ou de type de référentiel.

Votre produit peut simplement déléguer à votre méthode de révision statique, auquel cas vous fournissez une interface pratique dans un emplacement naturel (Product.getReviews), mais vos détails d'implémentation se trouvent dans Review.getForProduct.

SOLID sont des directives et devraient aboutir à une interface simple et sensée. Alternativement, vous pouvez dériver SOLID à partir d'interfaces simples et sensibles. Il s'agit de la gestion des dépendances dans le code. L'objectif est de minimiser les dépendances qui créent des frictions et créent des obstacles à des changements inévitables.

J'ai un point de vue légèrement différent à ce sujet que la plupart des autres réponses. Je pense que Productet Reviewsont essentiellement des objets de transfert de données (DTO). Dans mon code, j'essaie de faire en sorte que mes DTO / Entités évitent d'avoir des comportements. Ils sont juste une belle API pour stocker l'état actuel de mon modèle.

Lorsque vous parlez d'OO et de SOLID, vous parlez généralement d'un "objet" qui ne représente pas l'état (nécessairement), mais représente plutôt une sorte de service qui répond à vos questions ou auquel vous pouvez déléguer une partie de votre travail. . Par exemple:

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; }
}

Ensuite, votre réel ProductRepositoryretournerait un ExistingProductpour la GetProductByProductIdméthode, etc.

Vous suivez maintenant le principe de la responsabilité unique (tout ce qui hérite ne IProductfait que s'accrocher à l'état, et tout ce qui hérite IProductRepositoryest responsable de savoir comment persister et réhydrater votre modèle de données).

Si vous modifiez votre schéma de base de données, vous pouvez modifier l'implémentation de votre référentiel sans modifier vos DTO, etc.

Donc, en bref, je suppose que je ne choisirais aucune de vos options. :)

Les méthodes statiques peuvent être plus difficiles à tester, mais cela ne signifie pas que vous ne pouvez pas les utiliser - il vous suffit de les configurer de sorte que vous n'ayez pas besoin de les tester.

Créez les deux méthodes product.GetReviews et Review.ForProduct d'une ligne qui ressemblent à quelque chose

new ReviewService().GetReviews(productID);

ReviewService contient le code le plus complexe et possède une interface conçue pour la testabilité, mais qui n'est pas exposée directement à l'utilisateur.

Si vous devez avoir une couverture à 100%, demandez à vos tests d'intégration d'appeler les méthodes produit / classe de révision.

Cela peut être utile si vous pensez à la conception d'API publique plutôt qu'à la conception de classes - dans ce contexte, une interface simple avec un regroupement logique est tout ce qui compte - la structure de code réelle n'a d'importance que pour les développeurs.