7
votes

Est-ce une odeur de code pour une méthode pour dépendre d'une autre?

Je refactore une classe de sorte que le code soit testable (à l'aide de nunit et de rhinomocks comme des cadres de test et d'isolations) et j'ai constaté que je me suis trouvé avec une méthode dépend d'une autre (c'est-à-dire qu'elle dépend de quelque chose qui est créé par cette autre méthode). Quelque chose comme ce qui suit: xxx

ce qui signifie que pour tester une non-sohipersonation , j'ai besoin de le configurer en appelant IMPERSONATE (IMPERSONATE A déjà plusieurs tests unitaires pour vérifier son comportement). Cela me méprise, mais en quelque sorte, il est logique du point de vue du code qui appelle à cette classe: xxx

je n'aurais pas travaillé ceci si Je n'ai pas essayé d'écrire un test de l'unité pour Undoimsonation et je me suis retrouvé avoir à configurer le test en appelant l'autre méthode publique. Alors, est-ce une mauvaise odeur et si oui, comment puis-je y travailler?


1 commentaires

Incidemment, cela montre un côté important (?) - Avoir-avantage des tests unitaires: cela vous fait penser à votre conception ... :-)


6 Réponses :


8
votes

Eh bien, il y a un peu trop peu de contexte à dire, il semble que _somepend doit être intitulé dans le constructeur.

Les champs d'initialisation d'une méthode d'instance sont un grand non pour moi. Une classe devrait être entièrement utilisable (c'est-à-dire que toutes les méthodes fonctionnent) dès sa construction; Donc, le (s) constructeur (s) doit donc initialiser toutes les variables d'instance. Voir par exemple La page sur Construction d'une seule étape dans Ward Cunningham's Wiki.

Les champs d'initialisation de la raison dans une méthode d'instance sont méchants sont principalement qu'il impose une commande implicite sur la manière dont vous pouvez appeler des méthodes. Dans votre cas, Themethodiwantototest fera différentes choses selon que Dostuff s'appelait en premier. Ce n'est généralement pas quelque chose qu'un utilisateur de votre classe attendrait, alors c'est mauvais: - (.

Cela dit, parfois ce type d'accouplement peut être inévitable (par exemple, si une méthode acquiert une ressource telle qu'une poignée de fichier et une autre méthode est nécessaire pour la publier). Mais même cela devrait être traité dans une méthode si possible.

Qu'est-ce qui s'applique à votre cas est difficile à dire sans plus de contexte.


3 commentaires

_SomePePend est simplement en train de mettre la classe dans un état particulier. Et donc il ne peut pas être initialisé dans le constructeur de classe


Eh bien, avoir une classe avec différents états me douteux pour la même raison (la même méthode fera différentes choses en fonction de cet "état"). Difficile de raconter le contexte sans problème, mais votre classe est-elle peut-être trop en train de faire trop? Peut-être qu'une classe par "état" est plus appropriée (sous-classes possibles d'une certaine classe abstraite)?


J'ai édité le code pour espérer donner une meilleure idée du contexte.



2
votes

À condition que vous ne considérez pas d'objets mutables, une odeur de code par eux-mêmes, de mettre un objet dans l'état nécessaire à un test fait simplement partie de la configuration de cet essai.


0 commentaires

9
votes

L'odeur de code doit être l'un des termes les plus vagues que j'ai rencontrés dans le monde de la programmation. Pour un groupe de personnes qui se fient sur des principes d'ingénierie, il se classe à la hauteur de déchets non assurables, et à peu près comme une mesure inutile, en tant que LOCS par jour pour l'efficacité du programmeur.

Quoi qu'il en soit, c'est ma déclaration, merci d'avoir écouté: -)

Pour répondre à votre question spécifique, je ne crois pas que ce est un problème. Si vous testez quelque chose qui a des pré-conditions, vous devez vous assurer que les conditions préalables ont été configurées d'abord pour le cas de test donné.

L'un des tests devrait être ce qui se passe lorsque vous l'appelez sans d'abord mettre en place les pré-conditions - il devrait soit échouer gracieusement ou configurer sa propre condition si l'appelant n'a pas dérangé de le faire.


3 commentaires

Hm, i faire trouver des odeurs de code très utiles. Ce ne sont que des règles de pouce bien sûr, mais de bonnes (la plus au moins). Mais oui, si vous acceptez que la condition préalable est correcte (une question de conception), le test est correct. +1 pour les conseils pour tester le comportement sans configuration.


Je pense que la chose "odeur de code" souffre de la faille de programmeur habituelle de prendre tout trop littéralement.


+1 pour ajouter un test pour appeler la méthode sans remplir sa préconditionnement



2
votes

Ceci est souvent inévitable, par exemple lorsque vous travaillez avec des connexions distantes - vous devez appeler ouvert () avant de pouvoir appeler ferme () , et vous ne le faites pas veulent ouvert () pour arriver automatiquement dans le constructeur.

Cependant, vous voulez faire très attention lorsque le motif est facilement compris - par exemple, je pense que la plupart des utilisateurs acceptent ce type de comportement pour quoi que ce soit de transaction, mais pourraient être surpris quand ils rencontrent dostuff () < / code> et themethodiwantotestest () (quoi qu'ils soient vraiment appelés).

Il est normalement la meilleure pratique d'avoir une propriété qui représente l'état actuel - examinez à nouveau des connexions à distance ou à dB pour un exemple de conception constante.

Le gros no-non est pour cela que cela se produise pour les propriétés. Les propriétés doivent jamais les soins à quel ordre ils sont appelés. Si vous avez une valeur simple qui dépend de l'ordre des méthodes, il devrait s'agir d'une méthode sans paramétrage au lieu d'une propriété-get.


0 commentaires

1
votes

Pour répondre au titre: La méthode s'appelle l'un l'autre (chaînage) est inévitable dans la programmation orientée objet, donc à mon avis, il n'y a rien de mal à tester une méthode qui appelle une autre. Un test unitaire peut être une classe après tout, c'est une "unité" que vous testez.

Le niveau de chaînage dépend de la conception de votre objet - vous pouvez soit fourche ou cascade .

  • Forking: classotestest1.SomeDePendance.Dosomething ()
  • cascade : Classtotestest1.Dosomething () (qui appellerait en interne une somedpendency.dosomething)

    Mais comme d'autres personnes ont mentionné, gardez définitivement l'initialisation de votre état dans le constructeur qui, d'après ce que je peux dire, résoudra probablement votre problème.


0 commentaires

2
votes

Oui, je pense qu'il y a une odeur de code dans ce cas. Pas à cause de dépendances entre méthodes, mais à cause de l'identité vague de l'objet. Plutôt que d'avoir un impersonnateur qui peut être dans des États personnalisés différents, pourquoi ne pas avoir un personnalisé immuable ?

Si vous avez besoin d'un autre Persona , en créer un nouveau plutôt que de changer l'état d'un objet existant. Si vous devez faire un peu de nettoyage après, faites Persona Jetable. Vous pouvez conserver la classe impersonnator comme une usine: xxx


1 commentaires

+1 bon point. N'ayez pas peur de créer de nouvelles instances d'objet, même si elles sont jetables. Les électrons sont recyclables :-).