Refactoring Legacy Code Partie 9 - Analyse des problèmes

Dans ce tutoriel, nous allons continuer à nous concentrer sur notre logique métier. Nous évaluerons si RunnerFunctions.php appartient à une classe et si oui à quelle classe? Nous allons réfléchir aux problèmes et aux méthodes à utiliser. Enfin, nous en apprendrons un peu plus sur le concept de moquerie. Alors qu'est-ce que tu attends? Continuer à lire.


Fonctions Runner - De procédure à objet orienté

Bien que la plupart de notre code soit sous forme orientée objet, bien organisée en classes, certaines fonctions sont simplement assises dans un fichier. Nous devons en prendre pour donner les fonctions RunnerFunctions.php dans un aspect plus orienté objet.

const WRONG_ANSWER_ID = 7; const MIN_ANSWER_ID = 0; const MAX_ANSWER_ID = 9; function isCurrentAnswerCorrect ($ minAnswerId = MIN_ANSWER_ID, $ maxAnswerId = MAX_ANSWER_ID) return rand ($ minAnswerId, $ maxAnswerId)!! = WRONG_ANSWER_ID;  function run () $ display = new CLIDisplay (); $ aGame = new Game ($ display); $ aGame-> add ("Chet"); $ aGame-> add ("Pat"); $ aGame-> add ("Sue"); est-ce que $ dés = rand (0, 5) + 1; $ aGame-> roll ($ dés);  while (! didSomebodyWin ($ aGame, isCurrentAnswerCorrect ()));  function didSomebodyWin ($ aGame, $ isCurrentAnswerCorrect) if ($ isCurrentAnswerCorrect) return! $ aGame-> wasCorrectlyAnswered ();  else return! $ aGame-> wrongAnswer (); 

Mon premier instinct est de simplement les envelopper dans une classe. Ce n’est pas du génie, mais c’est quelque chose qui nous fait changer les choses. Voyons si l'idée peut réellement fonctionner.

const WRONG_ANSWER_ID = 7; const MIN_ANSWER_ID = 0; const MAX_ANSWER_ID = 9; class RunnerFunctions function isCurrentAnswerCorrect ($ minAnswerId = MIN_ANSWER_ID, $ maxAnswerId = MAX_ANSWER_ID) return rand ($ minAnswerId, $ maxAnswerId)! = WRONG_ANSWER_ID;  function run () //… // function didSomebodyWin ($ aGame, $ isCurrentAnswerCorrect) //… //

Si nous faisons cela, nous devons modifier nos tests et notre GameRunner.php utiliser la nouvelle classe. Nous avons appelé la classe quelque chose de générique pour le moment, il sera facile de le renommer en cas de besoin. Nous ne savons même pas si cette classe existera d'elle-même ou sera assimilée à Jeu. Alors ne vous inquiétez pas pour nommer pour l'instant.

fonction privée generateOutput ($ seed) ob_start (); srand ($ graine); (new RunnerFunctions ()) -> run (); $ output = ob_get_contents (); ob_end_clean (); return $ output; 

Dans notre GoldenMasterTest.php fichier, nous devons modifier la façon dont nous exécutons notre code. La fonction est generateOutput () et sa troisième ligne doit être modifiée pour créer un nouvel objet et appeler courir() dessus. Mais cela échoue.

Erreur irrécupérable PHP: appel de la fonction non définie didSomebodyWin () dans… 

Nous devons maintenant modifier notre nouvelle classe plus loin.

est-ce que $ dés = rand (0, 5) + 1; $ aGame-> roll ($ dés);  while (! $ this-> didSomebodyWin ($ aGame, $ this-> isCurrentAnswerCorrect ()));

Nous avions seulement besoin de changer la condition de la tandis que déclaration dans le courir() méthode. Les nouveaux appels de code didSomebodyWin () et isCurrentAnswerCorrect () de la classe en cours, en ajoutant $ this-> pour eux.

Cela fait passer le maître en or, mais freine les tests du coureur.

Erreur irrécupérable PHP: l'appel de la fonction non définie estCurrentAnswerCorrect () dans /… /RunnerFunctionsTest.php à la ligne 25

Le problème est dans assertAnswersAreCorrectFor (), mais facilement réparable en créant d'abord un objet runner.

fonction privée assertAnswersAreCorrectFor ($ correctAnserIDs) $ runner = new RunnerFunctions (); foreach ($ correctAnserIDs comme $ id) $ this-> assertTrue ($ runner-> isCurrentAnswerCorrect ($ id, $ id)); 

Ce même problème doit également être traité dans trois autres fonctions..

fonction testItCanFindWrongAnswer () $ runner = new RunnerFunctions (); $ this-> assertFalse ($ runner-> isCurrentAnswerCorrect (WRONG_ANSWER_ID, WRONG_ANSWER_ID));  function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ runner = new RunnerFunctions (); $ this-> assertTrue ($ runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aCorrectAnswer ()));  function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ runner = new RunnerFunctions (); $ this-> assertFalse ($ runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aWrongAnswer ())); 

Bien que cela fasse passer le code, cela introduit un peu de duplication de code. Comme nous en sommes maintenant à tous les tests sur le vert, nous pouvons extraire la création du coureur dans un fichier. installer() méthode.

coureur privé; fonction setUp () $ this-> runner = new Runner ();  function testItCanFindCorrectAnswer () $ this-> assertAnswersAreCorrectFor ($ this-> getCorrectAnswerIDs ());  function testItCanFindWrongAnswer () $ this-> assertFalse ($ this-> runner-> isCurrentAnswerCorrect (WRONG_ANSWER_ID, WRONG_ANSWER_ID));  function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ this-> assertTrue ($ this-> runner-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aCorrectAnswer ()));  function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ this-> assertFalse ($ this-> coureur-> didSomebodyWin ($ this-> aFakeGame (), $ this-> aWrongAnswer ());  fonction privée assertAnswersAreCorrectFor ($ correctAnserIDs) foreach ($ correctAnserIDs en tant que $ id) $ this-> assertTrue ($ this-> runner-> isCurrentAnswerCorrect ($ id, $ id)); 

Agréable. Toutes ces nouvelles créations et refactorings m'ont fait réfléchir. Nous avons nommé notre variable coureur. Peut-être que notre classe pourrait s'appeler la même chose. Refactorisons le. Ça devrait être facile.

Si vous n'avez pas vérifié "Rechercher des occurrences de texte"dans la case ci-dessus, n'oubliez pas de modifier manuellement vos inclus, car le refactoring renommera également le fichier.

Maintenant nous avons un fichier appelé GameRunner.php, un autre nommé Runner.php et un troisième nommé Game.php. Je ne sais pas pour vous, mais cela me semble extrêmement déroutant. Si je devais voir ces trois fichiers pour la première fois de ma vie, je n'aurais aucune idée de qui fait quoi. Nous devons nous débarrasser d'au moins un d'entre eux.

La raison pour laquelle nous avons créé le RunnerFunctions.php dans les premières étapes de notre refactoring, consistait à créer un moyen d’inclure toutes les méthodes et tous les fichiers à tester. Nous avions besoin d'accéder à tout, mais pas de tout faire sauf dans un environnement préparé dans notre maître d'or. Nous pouvons toujours faire la même chose, mais ne pas exécuter notre code à partir de GameRunner.php. Nous devons mettre à jour les includes et créer une classe à l'intérieur avant de continuer.

require_once __DIR__. '/Display.php'; require_once __DIR__. '/Runner.php'; (new Runner ()) -> run ();

Ça va le faire. Nous devons inclure Display.php explicitement, alors quand Coureur essaie de créer un nouveau CLIDisplay, il saura quoi mettre en œuvre.


Analyser les préoccupations

Je crois que l’une des caractéristiques les plus importantes de la programmation orientée objet est la définition des préoccupations. Je me pose toujours des questions telles que "cette classe fait-elle ce que son nom dit?", "Cette méthode concerne-t-elle cet objet?", "Mon objet doit-il tenir compte de cette valeur spécifique?"

Étonnamment, ces types de questions ont un grand pouvoir de clarification du domaine métier et de l’architecture logicielle. Nous posons et répondons à ce type de questions en groupe chez Syneto. Souvent, lorsqu'un programmeur a un dilemme, il se lève simplement et demande à l'équipe d'attirer l'attention pendant deux minutes afin de trouver notre opinion sur un sujet. Ceux qui sont familiers avec l’architecture du code répondront d’un point de vue logiciel, alors que d’autres, plus familiers avec le domaine des affaires, pourront éclairer certaines informations essentielles sur les aspects commerciaux..

Essayons de réfléchir aux problèmes dans notre cas. Nous pouvons continuer à nous concentrer sur Coureur classe. Il est extrêmement plus probable d'éliminer ou de transformer cette classe que Jeu.

Tout d’abord, un coureur doit-il se soucier de savoir comment isCurrentAnswerCorrect () travail? Si un coureur a des connaissances sur les questions et réponses?

Il semble vraiment que cette méthode serait mieux dans Jeu. Je crois fermement qu'un Jeu sur trivia devrait se soucier de savoir si une réponse est correcte ou non. Je crois vraiment un Jeu doit être soucieux de fournir le résultat de la réponse à la question actuelle.

Il est temps d'agir. Nous ferons un méthode de déplacement refactoring. Comme nous l'avons vu précédemment dans mes précédents tutoriels, je vais simplement vous montrer le résultat final..

require_once __DIR__. '/CLIDisplay.php'; include_once __DIR__. '/Game.php'; class Runner function run () //… // fonction didSomebodyWin ($ aGame, $ isCurrentAnswerCorrect) //… //

Il est essentiel de noter que non seulement la méthode a disparu, mais la constante qui définit les limites de la réponse.

Mais qu'en est-il didSomebodyWin ()? Un coureur doit-il décider quand quelqu'un a gagné? Si nous regardons le corps de la méthode, nous pouvons voir un problème mettant en évidence comme une lampe de poche dans le noir.

function didSomebodyWin ($ aGame, $ isCurrentAnswerCorrect) if ($ isCurrentAnswerCorrect) return! $ aGame-> wasCorrectlyAnswered ();  else return! $ aGame-> wrongAnswer (); 

Quoi que cette méthode fasse, elle le fait sur un Jeu objet seulement. Il vérifie la réponse actuelle renvoyée par partie. Ensuite, il retourne quel que soit l'objet de jeu retourné dans son wasCorrectlyAnswered () ou mauvaise réponse() méthodes. Cette méthode ne fait effectivement rien par elle-même. Tout ce qui compte c'est Jeu. C’est un exemple classique d’une odeur de code appelée Envie de fonctionnalité. Une classe fait quelque chose qu'une autre classe devrait faire. Il est temps de le déplacer.

class RunnerFunctionsTest étend PHPUnit_Framework_TestCase private $ runner; fonction setUp () $ this-> runner = new Runner (); 

Comme d'habitude, nous avons tout d'abord déplacé les tests. TDD? N'importe qui?

Cela ne nous laisse plus de tests à exécuter, ce fichier peut donc maintenant disparaître. La suppression est ma partie préférée de la programmation.

Et quand on lance nos tests, on obtient une belle erreur.

Erreur fatale: appel de la méthode non définie Game :: didSomebodyWin ()

Il est maintenant temps de changer le code également. Copier et coller la méthode dans Jeu va faire passer tous les tests comme par magie. Les anciens et les déplacés vers GameTest. Mais si cela met la méthode au bon endroit, il y a deux problèmes: le coureur doit également être changé et nous envoyons un faux Jeu objet que nous n'avons plus besoin de faire puisqu'il fait partie de Jeu.

est-ce que $ dés = rand (0, 5) + 1; $ aGame-> roll ($ dés);  while (! $ aGame-> didSomebodyWin ($ aGame, $ this-> isCurrentAnswerCorrect ()));

La fixation du coureur est très facile. Nous venons de changer $ this-> didSomebodyWin (…) dans $ aGame-> didSomebodyWin (…). Nous devrons revenir ici et le changer à nouveau, après notre prochaine étape. Le refactoring du test.

fonction testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided () $ aGame = \ Mockery :: mock ('Game [wasCorrectlyAnswered]'); $ aGame-> shouldReceive ('wasCorrectlyAnswered') -> once () -> andReturn (false); $ this-> assertTrue ($ aGame-> didSomebodyWin ($ this-> aCorrectAnswer ())); 

Il est temps de se moquer! Au lieu d'utiliser notre classe de faux, définie à la fin de nos tests, nous utiliserons Mockery. Cela nous permet d’écraser facilement une méthode sur Jeu, attendez-vous à être appelé et renvoyez la valeur que vous voulez. Bien sûr, nous pourrions y parvenir en faisant en sorte que notre classe de faux s'étende Jeu et écrasez la méthode nous-mêmes. Mais pourquoi faire un travail pour lequel un outil existe?

fonction testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided () $ aGame = \ Mockery :: mock ('Game [wrongAnswer]'); $ aGame-> shouldReceive ('wrongAnswer') -> once () -> andReturn (true); $ this-> assertFalse ($ aGame-> didSomebodyWin ($ this-> aWrongAnswer ())); 

Après la réécriture de notre deuxième méthode, nous pouvons nous débarrasser de la fausse classe de jeu et de toutes les méthodes qui l’ont initialisée. Problèmes résolus!

Dernières pensées

Même si nous avons réussi à ne penser qu'aux Coureur, nous avons fait de grands progrès aujourd'hui. Nous avons appris sur les responsabilités, nous avons identifié des méthodes et des variables appartenant à une autre classe. Nous avons réfléchi à un niveau supérieur et nous avons évolué vers une meilleure solution. Dans l’équipe Syneto, il existe une forte conviction qu’il existe des moyens de bien écrire le code et de ne jamais valider de changement, à moins que le code soit au moins un peu plus propre. C’est une technique qui à terme peut conduire à une base de code beaucoup plus agréable, avec moins de dépendances, plus de tests et finalement moins de bugs..

Merci pour votre temps.