Un «vrai travail» légitime chez un constructeur?

Je travaille sur un design, mais continue à frapper un barrage routier. J'ai une classe particulière (ModelDef) qui est essentiellement le propriétaire d'une arborescence de nœuds complexe construite en analysant un schéma XML (pensez DOM). Je veux suivre de bons principes de conception (SOLID) et m'assurer que le système résultant est facilement testable. J'ai bien l'intention d'utiliser DI pour passer des dépendances dans le constructeur de ModelDef (afin que celles-ci puissent facilement être échangées, si besoin est, pendant les tests).

Ce avec quoi je lutte, cependant, c'est la création de l'arborescence des nœuds. Cet arbre va être entièrement constitué d'objets simples de "valeur" qui n'auront pas besoin d'être testés indépendamment. (Cependant, je peux toujours passer une usine abstraite dans ModelDef pour aider à la création de ces objets.)

Mais je continue de lire qu'un constructeur ne devrait pas faire de vrai travail (par exemple Flaw: Constructor does Real Work ). Cela est parfaitement logique pour moi si le «vrai travail» signifie la construction d'objets dépendants de poids lourds que l'on pourrait plus tard vouloir extraire pour les tester. (Ceux-ci doivent être transmis via DI.)

Mais qu'en est-il des objets de valeur légers tels que cet arbre de nœuds? L'arbre doit être créé quelque part, non? Pourquoi pas via le constructeur de ModelDef (en utilisant, par exemple, une méthode buildNodeTree ())?

Je ne veux pas vraiment créer l'arborescence des nœuds en dehors de ModelDef, puis la transmettre (via le constructeur DI), car la création de l'arborescence des nœuds en analysant le schéma nécessite une quantité importante de code complexe - du code qui doit être soigneusement testé . Je ne veux pas le reléguer au code "glue" (qui devrait être relativement trivial et ne sera probablement pas testé directement).

J'ai pensé à mettre le code pour créer l'arborescence des nœuds dans un objet "builder" séparé, mais j'hésite à l'appeler un "builder", car il ne correspond pas vraiment au modèle Builder (qui semble plus soucieux d'éliminer le télescopage constructeurs). Mais même si je l'ai appelé quelque chose de différent (par exemple NodeTreeConstructor), cela ressemble toujours à un peu un hack juste pour éviter que le constructeur ModelDef construise l'arborescence des nœuds. Il doit être construit quelque part; pourquoi pas dans l'objet qui va le posséder?

Et, outre ce que Ross Patterson a suggéré, considérez cette position qui est exactement le contraire:

1. Prenez des maximes telles que «Tu ne feras aucun travail réel dans tes constructeurs» avec un grain de sel.

2. Un constructeur n'est vraiment qu'une méthode statique. Donc, structurellement, il n'y a vraiment pas beaucoup de différence entre:

   a) un constructeur simple et un tas de méthodes d'usine statiques complexes, et

   b) un constructeur simple et un tas de constructeurs plus complexes.

Une partie considérable du sentiment négatif à l'égard de tout travail réel chez les constructeurs vient d'une certaine période de l'histoire du C ++, alors qu'il y avait un débat sur précisément dans quel état l'objet sera laissé si une exception est levée dans le constructeur, et si le destructeur doit être invoqué dans un tel événement. Cette partie de l'histoire de C ++ est terminée et le problème a été résolu, alors que dans des langages comme Java, il n'y a jamais eu de problème de ce type pour commencer.

Mon avis est que si vous évitez simplement d'utiliser newdans le constructeur, (comme l'indique votre intention d'employer l'injection de dépendance), tout devrait bien se passer. Je ris des déclarations comme "la logique conditionnelle ou en boucle dans un constructeur est un signe d'avertissement d'une faille".

Outre tout cela, personnellement, je retirerais la logique d'analyse XML du constructeur, non pas parce qu'il est mauvais d'avoir une logique complexe dans un constructeur, mais parce qu'il est bon de suivre le principe de la "séparation des préoccupations". Donc, je déplacerais la logique d'analyse XML dans une classe distincte, pas dans certaines méthodes statiques qui appartiennent à votre ModelDefclasse.

Amendement

Je suppose que si vous avez une méthode en dehors de ModelDeflaquelle crée un ModelDefXML, vous devrez instancier une structure de données d'arborescence temporaire dynamique, la remplir en analysant votre XML, puis créer votre nouvelle ModelDeftransmission en passant cette structure en tant que paramètre constructeur. Donc, cela pourrait peut-être être considéré comme une application du modèle "Builder". Il y a une analogie très étroite entre ce que vous voulez faire et la paire String& StringBuilder. Cependant, j'ai trouvé ce Q & A qui semble être en désaccord, pour des raisons qui ne sont pas claires pour moi: Stackoverflow - StringBuilder et Builder Pattern . Donc, pour éviter un long débat ici sur la question de savoir si le StringBuildermodèle "constructeur" est mis en œuvre ou non, je dirais que vous pouvez vous inspirer de la façon dontStrungBuilder travaille à trouver une solution qui convient à vos besoins, et reporter l'appeler une application du modèle "Builder" jusqu'à ce que ce petit détail a été réglé.

Voir cette toute nouvelle question: Programmeurs SE: «StringBuilder» est-il une application du modèle de conception Builder?

Vous donnez déjà les meilleures raisons de ne pas faire ce travail dans le ModelDefconstructeur:

1. Il n'y a rien de «léger» dans l'analyse d'un document XML dans une arborescence de nœuds.
2. Il n'y a rien d'évident dans un ModelDefqui dit qu'il ne peut être créé qu'à partir d'un document XML.

Il semble que votre classe devrait avoir une variété de méthodes statiques comme ModelDef.FromXmlString(string xmlDocument), ModelDef.FromXmlDoc(XmlDoc parsedNodeTree), etc.

J'ai déjà entendu cette "règle". D'après mon expérience, c'est à la fois vrai et faux.

Dans une orientation d'objet plus "classique", nous parlons d'objets encapsulant l'état et le comportement. Ainsi, un constructeur d'objets doit s'assurer que l'objet est initialisé dans un état valide (et signaler un défaut si les arguments fournis ne rendent pas l'objet valide). S'assurer qu'un objet est initialisé dans un état valide me semble être un vrai travail. Et cette idée a des mérites, si vous avez un objet qui permet uniquement l'initialisation à un état valide via le constructeur et que l'objet encapsule correctement son état afin que chaque méthode qui change l'état vérifie également qu'elle ne change pas l'état en quelque chose de mauvais ... alors cet objet garantit par essence qu'il est "toujours valide". C'est une très belle propriété!

Donc, le problème survient généralement lorsque nous essayons de tout casser en petits morceaux pour tester et simuler des trucs. Parce que si un objet est vraiment correctement encapsulé, vous ne pouvez pas vraiment y entrer et remplacer le FooBarService par votre FooBarService simulé et vous (probablement) ne pouvez pas simplement changer les valeurs à volonté pour convenir à vos tests (ou cela peut prendre un beaucoup plus de code qu’une simple affectation).

Ainsi, nous obtenons l'autre «école de pensée», qui est SOLIDE. Et dans cette école de pensée, il est probablement beaucoup plus vrai que nous ne devrions pas faire un vrai travail chez le constructeur. Le code SOLIDE est souvent (mais pas toujours) plus facile à tester. Mais il peut aussi être plus difficile de raisonner. Nous séparons notre code en petits objets avec une seule responsabilité, et donc la plupart de nos objets n'encapsulent plus leur état (et contiennent généralement soit un état, soit un comportement). Le code de validation est généralement extrait dans une classe de validateur et conservé séparément de l'état. Mais maintenant que nous avons perdu la cohésion, nous ne pouvons plus être sûrs que nos objets sont valides lorsque nous les obtenons et pour être complètementbien sûr, nous devons toujours valider que les conditions préalables que nous pensons avoir sur l'objet sont vraies avant d'essayer de faire quelque chose avec l'objet. (Bien sûr, en général, vous effectuez la validation dans un seul calque, puis supposez que l'objet est valide dans les calques inférieurs.) Mais c'est plus facile à tester!

Alors, qui a raison?

Personne vraiment. Les deux écoles de pensée ont leurs mérites. Actuellement, SOLID fait fureur et tout le monde parle de SRP et d'Open / Closed et de tout ce jazz. Mais ce n'est pas parce que quelque chose est populaire que c'est le bon choix de conception pour chaque application. Cela dépend donc. Si vous travaillez dans une base de code qui suit fortement les principes SOLID, alors oui, un vrai travail dans le constructeur est probablement une mauvaise idée. Mais sinon, regardez la situation et essayez d'utiliser votre jugement. Quelles propriétés votre objet gagne-t-il en travaillant dans le constructeur, quelles propriétés perd- il ? Dans quelle mesure s'intègre-t-il à l'architecture globale de votre application?

Le vrai travail dans le constructeur n'est pas un contre-modèle, il peut être tout à fait le contraire lorsqu'il est utilisé aux bons endroits. Mais il doit être documenté clairement (avec quelles exceptions peuvent être levées, le cas échéant) et comme pour toute décision de conception - il doit correspondre au style général utilisé dans la base de code actuelle.

Il y a un problème fondamental avec cette règle et c'est ça, qu'est -ce qui constitue du "vrai travail"?

Vous pouvez voir dans l' article original publié dans la question que l'auteur tente de définir ce qu'est un «vrai travail», mais il est gravement imparfait. Pour qu'une pratique soit bonne, elle doit être un principe bien défini. J'entends par là qu'en ce qui concerne l'ingénierie logicielle, l'idée devrait être portable (agnostique dans n'importe quelle langue), testée et éprouvée. La plupart des arguments avancés dans cet article ne correspondent pas à ce premier critère. Voici quelques indicateurs que l'auteur mentionne dans cet article de ce qui constitue un «vrai travail» et pourquoi ce ne sont pas de mauvaises définitions.

Utilisation du newmot - clé . Cette définition est fondamentalement erronée car elle est spécifique au domaine. Certaines langues n'utilisent pas le newmot - clé. En fin de compte, il laisse entendre qu'il ne devrait pas être en train de construire d'autres objets. Cependant, dans de nombreuses langues, même les valeurs les plus élémentaires sont elles-mêmes des objets. Ainsi, toute valeur affectée dans le constructeur crée également un nouvel objet. Cela rend cette idée limitée à certaines langues, et un mauvais indicateur de ce qui constitue un "vrai travail".

Objet non complètement initialisé une fois le constructeur terminé . C'est une bonne règle, mais elle contredit également plusieurs des autres règles mentionnées dans cet article. Un bon exemple de la façon dont cela pourrait contredire les autres est mentionné la question qui m'a amené ici. Dans cette question, quelqu'un est préoccupé par l'utilisation de la sortméthode dans un constructeur dans ce qui semble être JavaScript en raison de ce principe. Dans cet exemple, la personne créait un objet qui représentait une liste triée d'autres objets. Aux fins de discussion, imaginez que nous avions une liste d'objets non triée et que nous avions besoin d'un nouvel objet pour représenter une liste triée. Nous avons besoin de ce nouvel objet car une partie de notre logiciel attend une liste triée et permet d'appeler cet objetSortedList. Ce nouvel objet accepte une liste non triée et l'objet résultant doit représenter une liste d'objets désormais triée. Si nous devions suivre les autres règles mentionnées dans ce document, à savoir aucun appel de méthode statique, aucune structure de flux de contrôle, rien de plus qu'une affectation, alors l'objet résultant ne serait pas construit dans un état valide en cassant l'autre règle de son initialisation complète après la fin du constructeur. Pour résoudre ce problème, nous aurions besoin de faire un travail de base pour trier la liste non triée dans le constructeur. Faire cela casserait les 3 autres règles, rendant les autres règles non pertinentes.

En fin de compte, cette règle de ne pas faire de «vrai travail» chez un constructeur est mal définie et défectueuse. Essayer de définir ce que le «vrai travail» est un exercice futile. La meilleure règle de cet article est que lorsqu'un constructeur termine, il doit être complètement initialisé. Il existe une pléthore d'autres meilleures pratiques qui limiteraient la quantité de travail effectuée dans un constructeur. La plupart de ceux-ci peuvent être résumés dans des principes SOLIDES, et ces mêmes principes ne vous empêcheraient pas de travailler dans le constructeur.

PS. Je me sens obligé de dire que même si j'affirme ici qu'il n'y a rien de mal à faire du travail chez le constructeur, ce n'est pas non plus le lieu de faire un tas de travail. SRP suggérerait qu'un constructeur devrait faire juste assez de travail pour le rendre valide. Si votre constructeur a trop de lignes de code (très subjectif je sais), alors il viole probablement ce principe et pourrait probablement être divisé en méthodes et objets plus petits et mieux définis.