Y a-t-il jamais une raison de faire tout le travail d'un objet dans un constructeur?

Permettez-moi de commencer par dire que ce n'est ni mon code ni celui de mes collègues. Il y a des années, lorsque notre société était plus petite, nous avions certains projets à réaliser dont nous n'avions pas la capacité, ils ont donc été externalisés. Maintenant, je n'ai rien contre l'externalisation ou les entrepreneurs en général, mais la base de code qu'ils ont produite est une masse de fichiers WTF. Cela étant dit, cela fonctionne (généralement), alors je suppose qu'il fait partie des 10% de projets externalisés les plus importants que j'ai vus.

Au fur et à mesure que notre société grandissait, nous avons essayé de prendre davantage en charge notre développement en interne. Ce projet particulier a atterri sur mes genoux, donc je l'ai examiné, nettoyé, ajouté des tests, etc.

Il y a un motif que je vois beaucoup se répéter et il semble tellement époustouflant que je me suis demandé s'il y avait peut-être une raison et que je ne la vois tout simplement pas. Le modèle est un objet sans méthodes ni membres publics, mais simplement un constructeur public qui effectue tout le travail de l'objet.

Par exemple, (le code est en Java, si cela compte, mais j'espère que ce sera une question plus générale):

public class Foo {
  private int bar;
  private String baz;

  public Foo(File f) {
    execute(f);
  }

  private void execute(File f) {
     // FTP the file to some hardcoded location, 
     // or parse the file and commit to the database, or whatever
  }
}

Si vous vous demandez, ce type de code est souvent appelé de la manière suivante:

for(File f : someListOfFiles) {
   new Foo(f);
}

Or, on m'a appris il y a longtemps que les objets instanciés dans une boucle sont généralement une mauvaise idée et que les constructeurs doivent effectuer un minimum de travail. En regardant ce code, il semble qu'il serait préférable de supprimer le constructeur et de créer executeune méthode statique publique.

J'ai demandé à l'entrepreneur pourquoi cela avait été fait ainsi, et la réponse que j'ai reçue était "Nous pouvons le changer si vous le souhaitez". Ce qui n'était pas vraiment utile.

Quoi qu'il en soit, y a-t-il déjà une raison de faire quelque chose comme cela, dans n'importe quel langage de programmation, ou s'agit-il simplement d'une autre soumission au Daily WTF?

Ok, en bas de la liste:

On m'a appris il y a longtemps que les objets instanciés dans une boucle sont généralement une mauvaise idée

Pas dans toutes les langues que j'ai utilisées.

En C, c’est une bonne idée de déclarer vos variables à l’avance, mais c’est différent de ce que vous avez dit. Cela peut être légèrement plus rapide si vous déclarez des objets au-dessus de la boucle et les réutilisez, mais il y a beaucoup de langages où cette augmentation de vitesse n'aura aucun sens (et probablement certains compilateurs qui optimisent pour vous :)).

En général, si vous avez besoin d'un objet dans une boucle, créez-en un.

les constructeurs devraient faire un minimum de travail

Les constructeurs doivent instancier les champs d'un objet et effectuer toute autre initialisation nécessaire pour le rendre prêt à être utilisé. Cela signifie généralement que les constructeurs sont petits, mais il existe des scénarios dans lesquels cela représenterait une charge de travail considérable.

y a-t-il jamais une raison de faire quelque chose comme cela, dans n'importe quel langage de programmation, ou s'agit-il simplement d'une autre soumission au Daily WTF?

C'est une soumission au Daily WTF. Certes, vous pouvez faire pire avec du code. Le problème est que l’auteur a une vaste incompréhension sur ce que sont les classes et comment les utiliser. Plus précisément, voici ce que je vois qui ne va pas avec ce code:

• Mauvais usage des classes: la classe agit essentiellement comme une fonction. Comme vous l'avez mentionné, elle devrait soit être remplacée par une fonction statique, soit simplement implémentée dans la classe qui l'appelle. Cela dépend de ce qu'il fait et de l'endroit où il est utilisé.
• Baisse des performances: Selon la langue, créer un objet peut être plus lent que d'appeler une fonction.
• Confusion générale: Il est généralement difficile pour le programmeur d’utiliser ce code. Sans le voir utilisé, personne ne saurait comment l'auteur avait l'intention d'utiliser le code en question.

Pour moi, il est un peu surprenant de voir un objet faire beaucoup de travail dans le constructeur, alors je vous recommande de ne pas le recommander (principe de la moindre surprise).

Une tentative de bon exemple :

Je ne sais pas si cette utilisation est un anti-motif, mais en C #, j'ai vu du code qui ressemblait à (exemple un peu maquillé):

using (ImpersonationContext administrativeContext = new ImpersonationContext(ADMIN_USER))
{
    // Perform actions here under the administrator's security context
}
// We're back to "normal" privileges here

Dans ce cas, la ImpersonationContextclasse effectue tout son travail dans le constructeur et la méthode Dispose. Il tire parti de l' usinginstruction C # , qui est assez bien comprise par les développeurs C #, et garantit ainsi que la configuration effectuée dans le constructeur est annulée dès que nous sommes en dehors de l' usinginstruction, même si une exception se produit. C’est probablement le meilleur usage que j’ai vu pour cela (c’est-à-dire "travail" dans le constructeur), bien que je ne sois pas encore sûr de le considérer comme "bon". Dans ce cas, c'est assez explicite, fonctionnel et succinct.

Un exemple de modèle correct et similaire serait le modèle de commande . Vous n'exécutez certainement pas la logique dans le constructeur ici, mais elle est similaire en ce sens que la classe entière est équivalente à une invocation de méthode (y compris les paramètres nécessaires pour appeler la méthode). De cette manière, les informations d’invocation de méthode peuvent être sérialisées ou transmises pour une utilisation ultérieure, ce qui est extrêmement utile. Peut-être que vos sous-traitants ont tenté quelque chose de similaire, bien que j'en doute fortement.

Commentaires sur votre exemple:

Je dirais que c'est un anti-modèle très précis. Il n’ajoute rien, va à l’encontre des attentes et effectue un traitement excessivement long dans le constructeur (FTP dans le constructeur? WTF, bien qu’il soit connu que les gens appellent les services Web de cette manière).

J'ai du mal à trouver la citation, mais le livre de Grady Booch "Object Solutions" contient une anecdote à propos d'une équipe de développeurs C en transition C ++. Apparemment, ils avaient suivi une formation et la direction leur avait dit qu'ils devaient absolument faire les choses «de manière orientée objet». Appelé à comprendre ce qui ne va pas, l'auteur a utilisé un outil de métrique de code et a constaté que le nombre moyen de méthodes par classe était exactement égal à 1 et constituait une variante de l'expression "Do It". Je dois dire que je n’avais jamais rencontré ce problème particulier auparavant, mais apparemment, il peut être symptomatique que des personnes soient forcées de créer du code orienté objet, sans vraiment comprendre comment le faire et pourquoi.

Non.

Si tout le travail est effectué dans le constructeur, cela signifie que l'objet n'a jamais été utilisé. Très probablement, l'entrepreneur utilisait simplement l'objet pour la modularité. Dans ce cas, une classe non instanciée avec une méthode statique aurait obtenu le même avantage sans créer d'instances d'objet superflues.

Il existe un seul cas dans lequel tout le travail dans un constructeur d'objet est acceptable. C’est lorsque l’objet a pour but spécifique de représenter une identité unique à long terme, plutôt que d’effectuer un travail. Dans un tel cas, l'objet serait conservé pour des raisons autres que l'exécution d'un travail significatif. Par exemple, une instance singleton.

J'ai certainement créé de nombreuses classes qui effectuent beaucoup de travail pour calculer quelque chose dans le constructeur, mais l'objet étant immuable, il rend les résultats de ce calcul disponibles via les propriétés, puis ne fait jamais rien d'autre. C'est l'immuabilité normale en action.

Je pense que la chose étrange ici est de mettre du code avec des effets secondaires dans un constructeur. Construire un objet et le jeter sans accéder aux propriétés ou aux méthodes semble étrange (mais au moins dans ce cas, il est assez évident que quelque chose d'autre se passe).

Ce serait mieux si la fonction était déplacée vers une méthode publique, puis qu'une instance de la classe soit injectée, puis que la boucle for appelle la méthode à plusieurs reprises.

Y a-t-il jamais une raison de faire tout le travail d'un objet dans un constructeur?

Non.

• Un constructeur ne devrait avoir aucun effet secondaire.
  • Tout ce qui dépasse l’initialisation du champ privé doit être considéré comme un effet secondaire.
  • Un constructeur avec des effets secondaires enfreint le principe de responsabilité unique (SRP) et va à l'encontre de l'esprit de la programmation orientée objet (OOP).
• Un constructeur doit être léger et ne doit jamais échouer.
  • Par exemple, je frémis toujours lorsque je vois un bloc try-catch dans un constructeur. Un constructeur ne doit pas lancer d’exceptions ni enregistrer les erreurs.

On peut raisonnablement remettre en question ces directives et dire: "Mais je ne respecte pas ces règles et mon code fonctionne bien!" À cela, je répondrais: "Cela peut être vrai jusqu'à ce que ce ne soit pas le cas."

• Les exceptions et les erreurs à l'intérieur d'un constructeur sont très inattendues. Sauf indication contraire de leur part, les futurs programmeurs ne seront pas enclins à entourer ces appels de constructeur de code défensif.
• Si quelque chose échoue en production, la trace de pile générée peut être difficile à analyser. Le haut de la trace de la pile peut pointer vers l'appel du constructeur, mais de nombreux événements se produisent dans le constructeur et il peut ne pas pointer vers le LOC réel ayant échoué.
  • J'ai analysé de nombreuses traces de pile .NET là où c'était le cas.

Il me semble que cette personne essayait de contourner la limitation de Java qui ne permet pas de transférer des fonctions en tant que citoyens de première classe. Chaque fois que vous devez passer une fonction, vous devez l'envelopper dans une classe avec une méthode similaire applyou similaire.

Donc, je suppose qu’il a juste utilisé un raccourci et utilisé directement une classe pour remplacer une fonction. Chaque fois que vous voulez exécuter la fonction, il vous suffit d'instancier la classe.

Ce n’est certainement pas un bon modèle, du moins parce que nous essayons de le comprendre, mais j’imagine que c’est peut-être la façon la moins verbeuse de faire quelque chose de similaire à la programmation fonctionnelle en Java.

Pour moi, la règle de base est de ne rien faire dans le constructeur, sauf pour l'initialisation des variables membres. La première raison à cela est qu'il est utile de suivre le principe SRP, car un long processus d'initialisation indique que la classe en fait plus que ce qu'elle devrait faire et que l'initialisation doit être faite quelque part en dehors de la classe. La deuxième raison est que de cette manière, seuls les paramètres nécessaires sont passés, vous créez donc du code moins couplé. Un constructeur avec une initialisation compliquée a généralement besoin de paramètres qui ne sont utilisés que pour construire un autre objet, mais qui ne sont pas utilisés par la classe d'origine.

Je vais aller à contre-courant ici - dans les langues où tout doit être dans une classe ET vous ne pouvez pas avoir une classe statique, vous pouvez vous retrouver dans une situation où vous avez un élément isolé de fonctionnalité qui doit être mettre quelque part et ne correspond à rien d'autre. Vos choix sont une classe d’utilité qui couvre divers éléments de fonctionnalité non liés, ou une classe qui fait essentiellement cette chose-là.

Si vous avez juste un bit comme celui-ci, alors votre classe statique est votre classe spécifique. Donc, soit vous avez un constructeur qui fait tout le travail, ou une seule fonction qui fait tout le travail. L'avantage de tout faire dans le constructeur est que cela ne se produit qu'une seule fois, cela vous permet d'utiliser des champs privés, des propriétés et des méthodes sans vous soucier de leur réutilisation ou de leur threading. Si vous avez un constructeur / une méthode, vous pouvez être tenté de laisser les paramètres être transmis à la méthode unique, mais si vous utilisez des champs privés ou des propriétés qui introduisent des problèmes de threads.

Même avec une classe utilitaire, la plupart des gens ne penseront pas à avoir des classes statiques imbriquées (et cela pourrait ne pas être possible non plus en fonction de la langue).

Fondamentalement, il s'agit d'un moyen relativement compréhensible d'isoler le comportement du reste du système, dans les limites autorisées par la langue, tout en vous permettant de tirer le meilleur parti possible de la langue.

Franchement, j'aurais répondu à peu près comme votre entrepreneur, c'est un ajustement mineur de le transformer en deux appels, il n'y a pas tant de choses à recommander l'un à l'autre. Quels inconvénients / avantages il y a, sont probablement plus hypothétiques que réels, pourquoi essayer de justifier la décision quand cela n'a pas d'importance et qu'il n'a probablement pas été tant décidé que l'action par défaut?

Il existe des modèles communs dans les langages où les fonctions et les classes sont beaucoup plus semblables, comme Python, où l’on utilise un objet pour fonctionner avec trop d’effets secondaires ou avec plusieurs objets nommés retournés.

Dans ce cas, la majeure partie, sinon la totalité du travail, peut être effectuée dans le constructeur, car l'objet lui-même est simplement une enveloppe autour d'un seul lot de travail, et il n'y a aucune bonne raison de le ranger dans une fonction distincte. Quelque chose comme

var parser = XMLParser()
var x = parser.parse(string)
string a = x.LookupTag(s)

est vraiment pas mieux que

 var x = ParsedXML(string)
 string a = x.LookupTag(s)

Au lieu d’avoir directement les effets secondaires, vous pouvez appeler l’objet pour imposer l’effet secondaire sur le repère, ce qui vous donne une meilleure visibilité. Si je vais avoir un effet secondaire néfaste sur un objet, je peux faire tout mon travail puis me faire passer l'objet que je vais mal faire et le blesser. Identifier l'objet et isoler l'appel de cette façon est plus clair que de passer plusieurs de ces objets à la fois dans une fonction.

x.UpdateView(v)

Vous pouvez rendre les valeurs de retour multiples via des accesseurs sur l'objet. Tout le travail est terminé, mais je veux que tout soit dans son contexte et je ne souhaite pas vraiment transmettre plusieurs références à ma fonction.

var x = new ComputeStatistics(inputFile)
int max = x.MaxOptions;
int mean = x.AverageOption;
int sd = x.StandardDeviation;
int k = x.Kurtosis;
...

Donc, en tant que programmeur mixte Python / C #, cela ne me semble pas si étrange.

Un cas vient à l’esprit où le constructeur / destructeur fait tout le travail:

Serrures Normalement, vous n'interagissez jamais avec un verrou pendant sa durée de vie. L’utilisation de l’architecture de C # permet de traiter de tels cas sans faire explicitement référence au destructeur. Cependant, tous les verrous ne sont pas implémentés de cette façon.

J'ai également écrit ce qui équivaut à un morceau de code réservé aux constructeurs pour corriger un bogue de bibliothèque d'exécution. (En réalité, corriger le code aurait causé d’autres problèmes.) Il n’y avait pas de destructeur car il n’était pas nécessaire d’annuler le correctif.

Je suis d'accord avec AndyBursh. Cela semble être un problème de manque de connaissances.

Cependant, il est important de mentionner les frameworks tels que NHibernate qui vous obligent à coder dans le constructeur (lors de la création de classes de carte). Cependant, il ne s'agit pas d'une limitation du cadre, mais de ce dont vous avez besoin. En ce qui concerne la réflexion, le constructeur est la seule méthode qui existera toujours (même si elle est privée) et cela pourrait être utile si vous devez appeler une méthode d'une classe de manière dynamique.

"On m'a appris il y a longtemps que les objets instanciés dans une boucle sont généralement une mauvaise idée"

Oui, vous vous rappelez peut-être pourquoi nous avons besoin de StringBuilder.

Il y a deux écoles de Java.

Le constructeur a été promu par le premier , ce qui nous apprend à réutiliser (parce que partager = identité à efficacité). Cependant, au début de l’année 2000, j’ai commencé à lire que la création d’objets en Java était si peu coûteuse (par opposition au C ++) et que GC était si efficace que sa réutilisation n’avait aucune valeur et que la seule chose qui importait était la productivité du programmeur. Pour la productivité, il doit écrire du code maniable, ce qui signifie la modularisation (ou réduire la portée). Alors,

c'est mauvais

byte[] array = new byte[kilos or megas]
for_loop:
  use(array)

c'est bon.

for_loop:
  byte[] array = new byte[kilos or megas]
  use(array)

J'ai posé cette question lorsque forum.java.sun existait et les gens ont convenu qu'ils préféreraient déclarer le tableau aussi étroit que possible.

Le programmeur Java moderne n'hésite jamais à déclarer une nouvelle classe ou à créer un objet. Il est encouragé à le faire. Java donne toutes les raisons de le faire, avec pour idée que "tout est un objet" et une création peu coûteuse.

En outre, dans le style de programmation d'entreprise moderne, lorsque vous devez exécuter une action, vous créez une classe spéciale (ou mieux encore, un framework) qui exécutera cette action simple. Bons IDEs Java, ça aide ici. Ils génèrent des tonnes de merde automatiquement, comme la respiration.

Donc, je pense que vos objets n’ont pas le motif Builder ou Factory. Il n'est pas convenable de créer des instances directement, par constructeur. Une classe d’usine spéciale pour chaque objet est conseillée.

Désormais, lorsque la programmation fonctionnelle entre en jeu, la création d'objets devient encore plus simple. Vous allez créer des objets à chaque étape en respirant, sans même vous en rendre compte. Donc, je m'attends à ce que votre code devienne encore plus "efficace" dans la nouvelle ère

"ne faites rien dans votre constructeur. C'est pour l'initialisation seulement"

Personnellement, je ne vois aucune raison dans la ligne directrice. Je considère que c'est juste une autre méthode. Le code d'affacturage n'est nécessaire que lorsque vous pouvez le partager.