La logique métier dans les constructeurs est-elle une bonne idée?

Question

Je suis actuellement en train de reconstruire un système de ticket spécialisé au travail (principalement utilisé pour soutenir les personnes ayant des défauts dans le matériel de télédétection ...). Quoi qu'il en soit, je me demandais si faire beaucoup d'activité de type workflow dans le constructeur d'un objet est une bonne idée.

Par exemple, il est actuellement ceci:

$ticket = new SupportTicket(
    $customer, 
    $title, 
    $start_ticket_now, 
    $mail_customer 
); 

dès que l'objet est créé, il va mettre une ligne dans une base de données, rendez-vous et l'envoyer au client un e-mail de confirmation, peut-être envoyer un message texte au technicien le plus proche, etc.

Est-ce qu'un constructeur doit déclencher tout ce travail, ou quelque chose comme ce qui suit?

$ticket = new SupportTicket($customer, $title); 
$customer->confirmTicketMailed($ticket); 
$helpdesk->alertNewTicket($ticket); 

Si elle aide, les objets sont tous basés sur le style ActiveRecord.

Je suppose que c'est peut-être une question d'opinion, mais que pensez-vous être la meilleure chose à faire?

Answer 1

Si le constructeur fait tout ce qui fonctionne alors le constructeur connaît de nombreux autres objets de domaine. Cela crée un problème de dépendance. Est-ce que le ticket devrait vraiment connaître le Customer et le HelpDesk? Lorsque de nouvelles fonctionnalités sont ajoutées, n'est-il pas probable que de nouveaux objets de domaine seront ajoutés au flux de travail, et cela ne signifie-t-il pas que notre pauvre ticket devra connaître une population sans cesse croissante d'objets de domaine?Le problème avec les toiles d'araignée de dépendance comme ceci est qu'un changement de code source à l'un des objets de domaine aura un impact sur nos pauvres ticket. Le ticket aura tellement connaissance du système que, quoi qu'il arrive, le ticket sera impliqué. Vous trouverez des instructions if désagréables regroupées dans ce constructeur, vérifiant la base de données de configuration et l'état de la session, et bien d'autres choses. Le ticket deviendra une classe divine.

Une autre raison pour laquelle je n'aime pas les constructeurs qui font des choses est que cela rend les objets autour d'eux très difficiles à tester. J'aime écrire beaucoup d'objets simulés. Quand j'écris un test contre le customer je veux le passer un ticket moqué. Si le constructeur de ticket contrôle le flux de travail, et la danse entre customer et d'autres objets de domaine, alors il est peu probable que je serai en mesure de se moquer pour tester le customer.

Je vous suggère de lire The SOLID Principles, un article que j'ai écrit il y a plusieurs années sur la gestion des dépendances dans les conceptions orientées objet.

Answer 2

Divisez les choses. Vraiment, vous ne voulez pas un ticket pour savoir comment il devrait envoyer un e-mail, etc. C'est le travail d'un service comme classe.

[Mise à jour] Je ne pense pas que les modèles d'usine suggérés sont bons pour cela. Une fabrique est utile si vous voulez créer différentes implémentations de tickets sans mettre cette logique dans le ticket lui-même (via des constructeurs surchargés par exemple).

Jetez un coup d'œil au concept de service proposé dans Domain-Driven Design.

services: Lorsqu'une opération ne fait pas partie conceptuelle à un objet. En suivant les contours naturels du problème, vous pouvez implémenter ces opérations dans les services. Le concept de service est appelé "Pure Fabrication" dans GRASP.

Answer 3

Le problème, du point de vue de la conception OO, n'est pas tant que devrait être mis en œuvre cette fonctionnalité dans un constructeur (par opposition à l'intérieur d'autres méthodes sur cette classe), mais si la classe Support de doit savoir comment faire toutes ces choses. En résumé, la classe SupportTicket devrait modéliser un ticket de support, et seulement un ticket de support. Créer un e-mail, savoir comment envoyer cet e-mail au client, mettre le ticket en file d'attente pour le traitement, etc., sont autant de fonctionnalités que vous devriez déplacer de votre classe SupportTicket et encapsuler ailleurs. Les avantages de cela comprennent un couplage plus faible, une cohésion plus élevée et une testabilité améliorée. Jetez un oeil à la Single Responsibility Principle, ce qui explique les avantages de le faire. En particulier, this blog est un bon endroit pour lire sur SRP et les autres principes clés de la bonne conception OO.

Answer 4

La réponse courte est non.

Dans la conception du matériel, nous avions l'habitude d'avoir un dicton, "Ne pas mettre une porte sur l'horloge ou la ligne de réinitialisation - il obscurcit la logique." La même chose s'applique ici pour la même raison. La réponse la plus longue devra attendre, mais voir "ScreetchinglyObvious Code".

Answer 5

En fait, je n'ai jamais été satisfait des réponses disponibles, mais regardons-les. Les choix sont construits autour de deux questions d'évaluation:

E1. D'où vient la connaissance de la logique métier?

E2. À quoi ressemblera le lecteur suivant du code? (Screechingly Obvious Code)

Quelques choix:

• Dans le code client (l'objet qui fait "nouveau Support de"). Il n'est probablement pas/ne devrait pas connaître la logique métier, évidemment, sinon vous ne voudriez pas créer ce constructeur fantaisiste. Si elle est le bon endroit pour la logique métier, alors il devrait dire:

  $ticket = new SupportTicket($customer, $title); 
  
  handleNewSupportTicket($ticket, ...other parameters...) 
  

  où, afin de protéger E2, « handlenewSupportTicket » est le lieu où cette logique métier est définie (et le prochain programmeur peut facilement trouve le).

• Dans l'objet ticket de support, en tant qu'appel d'affaire séparé. Personnellement, je ne suis pas vraiment content de ça, parce que c'est deux appels du code client, où la pensée mentale est une chose.

  $ticket = new SupportTicket($customer, $title); 
  
  $ticket -> handleNewSupportTicket(...other parameters...) 
  

• Dans la classe des billets de soutien.Ici, il est prévu que la logique métier réside dans la zone Support Ticket, mais comme de nouveaux tickets de support doivent absolument être traités immédiatement et non plus tard, cette tâche importante ne peut être laissée à l'imagination ou à l'erreur de quelqu'un, en particulier code client. Je ne sais comment le code des méthodes de classe dans Smalltalk, mais je vais prendre un coup de poignard au pseudo-code:

  $ticket = SupportTicket.createAndHandleNewSupportTicket(...other parameters...) 
  

  En supposant que le code client a besoin de la poignée du nouveau billet à d'autres fins (sinon le « $ ticket = "disparaîtrait". Je ne suis pas très friands de cela, car d'autres programmeurs ne trouvent pas si naturel de chercher une logique métier en classe ou des méthodes statiques. Mais c'est le troisième choix. Le seul quatrième choix est s'il y a un autre endroit où la logique métier réside joyeusement et d'autres programmeurs vont naturellement le chercher, auquel cas il passe dans une fonction classe/statique là-bas.

  $ticket = SupportTicketBusinessLogic.createAndHandleNewSupportTicket(...other params...) 
  

  où la fonction classe/statique effectue les appels multiples nécessaires. (Mais maintenant, nous avons une fois de plus la possibilité que les billets peuvent être construits et non manipulés correctement :(