Je lis des odeurs de code communes dans le livre Refactoring de Martin Fowler . Dans ce contexte, je me posais des questions sur un modèle que je vois dans une base de code, et si l'on pouvait objectivement le considérer comme un anti-modèle.
Le modèle est celui où un objet est passé en argument à une ou plusieurs méthodes, qui modifient toutes l'état de l'objet, mais aucune ne renvoie l'objet. Il s'appuie donc sur la nature pass by reference de (dans ce cas) C # /. NET.
var something = new Thing();
// ...
Foo(something);
int result = Bar(something, 42);
Baz(something);
Je trouve que (en particulier lorsque les méthodes ne sont pas nommées de manière appropriée), je dois examiner ces méthodes pour comprendre si l'état de l'objet a changé. Cela rend la compréhension du code plus complexe, car j'ai besoin de suivre plusieurs niveaux de la pile d'appels.
Je voudrais proposer d'améliorer ce code pour retourner un autre objet (cloné) avec le nouvel état, ou tout ce qui est nécessaire pour changer l'objet sur le site d'appel.
var something1 = new Thing();
// ...
// Let's return a new instance of Thing
var something2 = Foo(something1);
// Let's use out param to 'return' other info about the operation
int result;
var something3 = Bar(something2, out result);
// If necessary, let's capture and make explicit complex changes
var changes = Baz(something3)
something3.Apply(changes);
Il me semble que le premier modèle est choisi sur les hypothèses
- que c'est moins de travail, ou nécessite moins de lignes de code
- qu'il nous permet à la fois de changer l'objet et de renvoyer une autre information
- qu'il est plus efficace car nous avons moins d'instances de
Thing
.
J'illustre une alternative, mais pour la proposer, il faut avoir des arguments contre la solution originale. Quels arguments, le cas échéant, peuvent être avancés pour démontrer que la solution originale est un anti-modèle?
Et qu'est-ce qui ne va pas, le cas échéant, avec ma solution alternative?
la source
Réponses:
Oui, la solution d'origine est un anti-modèle pour les raisons que vous décrivez: il est difficile de raisonner sur ce qui se passe, l'objet n'est pas responsable de son propre état / implémentation (rupture de l'encapsulation). J'ajouterais également que tous ces changements d'état sont des contrats implicites de la méthode, ce qui rend cette méthode fragile face à l'évolution des exigences.
Cela dit, votre solution a certains de ses propres inconvénients, dont le plus évident est que le clonage d'objets n'est pas génial. Cela peut être lent pour les gros objets. Cela peut entraîner des erreurs lorsque d'autres parties du code s'accrochent aux anciennes références (ce qui est probablement le cas dans la base de code que vous décrivez). Rendre ces objets explicitement immuables résout au moins quelques-uns de ces problèmes, mais c'est un changement plus radical.
À moins que les objets ne soient petits et quelque peu transitoires (ce qui en fait de bons candidats pour l'immuabilité), je serais enclin à simplement déplacer davantage la transition d'état dans les objets eux-mêmes. Cela vous permet de masquer les détails d'implémentation de ces transitions et de définir des exigences plus strictes concernant qui / quoi / où ces transitions d'état se produisent.
la source
Bar(something)
(et de modifier l'état desomething
), créezBar
un membre desomething
type.something.Bar(42)
est plus susceptible de mutersomething
, tout en vous permettant également d'utiliser des outils OO (état privé, interfaces, etc.) pour protégersomething
l'état deEn fait, c'est la vraie odeur de code. Si vous avez un objet mutable, il fournit des méthodes pour changer son état. Si vous avez un appel à une telle méthode intégrée dans une tâche de quelques autres instructions, il est bon de refactoriser cette tâche à une méthode qui lui est propre - ce qui vous laisse exactement dans la situation décrite. Mais si vous n'avez pas de noms de méthode comme
Foo
etBar
, mais des méthodes qui indiquent clairement qu'ils changent l'objet, je ne vois pas de problème ici. Penser àou
ou
ou
ou quelque chose comme ça - je ne vois aucune raison ici de retourner un objet cloné pour ces méthodes, et il n'y a pas non plus de raison d'examiner leur implémentation pour comprendre qu'elles changeront l'état de l'objet passé.
Si vous ne voulez pas d'effets secondaires, rendez vos objets immuables, il appliquera des méthodes comme celles ci-dessus pour renvoyer un objet modifié (cloné) sans changer celui d'origine.
la source
Oui, voir http://codebetter.com/matthewpodwysocki/2008/04/30/side-effecting-functions-are-code-smells/ pour l'un des nombreux exemples de personnes soulignant que les effets secondaires inattendus sont mauvais.
En général, le principe fondamental est que le logiciel est construit en couches, et chaque couche doit présenter l'abstraction la plus propre possible à la suivante. Et une abstraction propre est celle où vous devez garder le moins possible à l'esprit pour l'utiliser. C'est ce qu'on appelle la modularité, et s'applique à tout, des fonctions simples aux protocoles en réseau.
la source
ForEach<T>
fait.Tout d'abord, cela ne dépend pas de la "nature passante de référence", cela dépend des objets qui sont des types de référence mutables. Dans les langages non fonctionnels, cela sera presque toujours le cas.
Deuxièmement, que ce soit un problème ou non, cela dépend à la fois de l'objet et de la façon dont les changements dans les différentes procédures sont liés - si vous ne faites pas de changement dans Foo et que Bar se bloque, alors c'est un problème. Pas nécessairement une odeur de code, mais c'est un problème avec Foo ou Bar ou Something (probablement Bar car il devrait vérifier son entrée, mais il pourrait s'agir de quelque chose mis dans un état invalide qu'il devrait empêcher).
Je ne dirais pas que cela s'élève au niveau d'un anti-pattern, mais plutôt quelque chose à savoir.
la source
Je dirais qu'il y a peu de différence entre
A.Do(Something)
modifiersomething
etsomething.Do()
modifiersomething
. Dans les deux cas, il doit être clair d'après le nom de la méthode invoquée quisomething
sera modifiée. Si le nom de la méthode n'est pas clair, qu'il s'agisse d'something
un paramètrethis
ou d'une partie de l'environnement, il ne doit pas être modifié.la source
Je pense que c'est bien de changer l'état de l'objet dans certains scénarios. Par exemple, j'ai une liste d'utilisateurs et je souhaite appliquer différents filtres à la liste avant de la renvoyer au client.
Et oui, vous pouvez rendre cela joli en déplaçant le filtrage dans une autre méthode, vous vous retrouverez donc avec quelque chose comme:
Où
Filter(users)
exécuterait les filtres ci-dessus.Je ne me souviens pas exactement où je suis tombé sur ce sujet auparavant, mais je pense qu'il était appelé pipeline de filtrage.
la source
Je ne sais pas si la nouvelle solution proposée (de copier des objets) est un modèle. Le problème, comme vous l'avez souligné, est la mauvaise nomenclature des fonctions.
Disons que j'écris une opération mathématique complexe en tant que fonction f () . Je documente que f () est une fonction qui correspond
NXN
àN
, et l'algorithme derrière elle. Si la fonction est nommée de manière inappropriée et n'est pas documentée, et n'a pas de cas de test d'accompagnement, et vous devrez comprendre le code, auquel cas le code est inutile.À propos de votre solution, quelques observations:
X
devenusY
aprèsf()
, mais leX
sont en faitY
) et éventuellement une incohérence temporelle.Le problème que vous essayez de résoudre est valide; cependant, même avec une énorme ingénierie excessive, le problème est contourné, pas résolu.
la source