Share
Dans notre leçon précédente, nous avons appris une nouvelle façon de comprendre et d'améliorer le code en extrayant jusqu'à la chute. Bien que ce tutoriel ait été un bon moyen d’apprendre les techniques, ce n’était pas un exemple idéal pour en comprendre les avantages. Dans cette leçon, nous allons extraire jusqu'à laisser tomber tout notre code relatif au jeu-questionnaire et analyser le résultat final..
Cette leçon conclura également notre série sur la refactorisation. Si vous pensez avoir oublié quelque chose, n'hésitez pas à commenter un sujet proposé. Si de bonnes idées se rassemblent, je continuerai avec des tutoriels supplémentaires en fonction de vos demandes..
Quelle meilleure façon de commencer notre article que de prendre notre méthode la plus longue et de l'extraire par petits morceaux. Comme d’habitude, le test en premier rendra cette procédure non seulement efficace, mais aussi amusante..
Comme d'habitude, vous avez le code tel qu'il était quand j'ai commencé ce tutoriel dans le php_start dossier, alors que le résultat final est dans la php dossier.
function wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ gagnant = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ gagnant; else $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return true; else $ this-> display-> correctAnswerWithTypo (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ gagnant = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ gagnant;
Cette méthode, wasCorrectlyAnswered (), est notre première victime.
Comme nous l’avons appris dans les leçons précédentes, la première étape lors de la modification du code hérité consiste à le tester. Cela peut être un processus difficile. Heureusement pour nous wasCorrectlyAnswered () la méthode est assez simple. Il est composé de plusieurs sinon déclarations. Chaque branche du code renvoie une valeur. Lorsque nous avons une valeur de retour, nous pouvons toujours supposer que le test est faisable. Pas nécessaire facile, mais au moins possible.
fonction testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> isGettingOutOfPenaltyBox = true; $ this-> game-> bourses [$ this-> game-> currentPlayer] = Game :: $ numberOfCoinsToWin; $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ());
Il n'y a pas de règle précise sur le test à écrire en premier. Nous venons de choisir le premier chemin d'exécution ici. En fait, nous avons eu une bonne surprise et nous avons réutilisé l’une des méthodes privées que nous avons extraites auparavant. Mais nous n'avons pas encore fini. Tout en vert, il est temps de refactoriser.
fonction testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerAWinner (); $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ());
C'est plus facile à lire et beaucoup plus descriptif. Vous pouvez trouver les méthodes extraites dans le code ci-joint.
fonction testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileNOTBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerNotAWinner (); $ this-> assertFalse ($ this-> game-> wasCorrectlyAnswered ()); fonction privée setCurrentPlayerNotAWinner () $ this-> jeu-> porte-monnaie [$ this-> jeu-> currentPlayer] = 0;
Nous nous attendions à ce que cela passe, mais cela échoue. Les raisons ne sont pas claires du tout. A regarder de plus près didPlayerNotWin () peut être utile.
fonction didPlayerNotWin () return! ($ this-> porte-monnaie [$ this-> currentPlayer] == self :: $ numberOfCoinsToWin);
La méthode retourne à vrai lorsqu'un joueur n'a pas gagné. Peut-être pourrions-nous renommer notre variable mais d’abord, les tests doivent réussir.
fonction privée setCurrentPlayerAWinner () $ this-> jeu-> porte-monnaie [$ this-> jeu-> currentPlayer] = Game :: $ numberOfCoinsToWin; fonction privée setCurrentPlayerNotAWinner () $ this-> jeu-> porte-monnaie [$ this-> jeu-> currentPlayer] = 0;
En regardant de plus près, nous pouvons voir que nous avons mélangé les valeurs ici. Notre confusion entre le nom de la méthode et le nom de la variable nous a fait inverser les conditions.
fonction privée setCurrentPlayerAWinner () $ this-> jeu-> porte-monnaie [$ this-> jeu-> currentPlayer] = 0; fonction privée setCurrentPlayerNotAWinner () $ this-> jeu-> porte-monnaie [$ this-> jeu-> currentPlayer] = Game :: $ numberOfCoinsToWin - 1;
Ça marche. En analysant didPlayerNotWin () nous avons également observé qu'il utilise l'égalité pour déterminer le gagnant. Nous devons définir notre valeur à un moins car la valeur est incrémentée dans le code de production que nous testons.
Les trois tests restants sont simples à écrire. Ce ne sont que des variantes des deux premiers. Vous pouvez les trouver dans le code ci-joint.
Le problème le plus déroutant est le trompeur gagnant de $ Nom de variable. Ça devrait être $ notAWinner.
$ notAWinner = $ this-> didPlayerNotWin (); $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ notAWinner;
Nous pouvons observer que le $ notAWinner variable est utilisée uniquement pour renvoyer une valeur. Pouvons-nous appeler le didPlayerNotWin () méthode directement dans l'instruction de retour?
$ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return $ this-> didPlayerNotWin ();
Cela réussit toujours notre test unitaire, mais si nous exécutons nos tests Golden Master, ils échoueront avec l'erreur "pas assez de mémoire". En fait, le changement rend le jeu jamais fini.
Qu'est-ce qui se passe est que le joueur actuel est mis à jour pour le prochain joueur. Comme nous n'avions qu'un seul joueur, nous avons toujours réutilisé le même joueur. C'est comme ça que les tests sont. Vous ne savez jamais quand vous découvrez une logique cachée dans un code difficile.
Accentsconagua
fonction testWasCorrectlyAnsweredAndGettingOutOfPenaltyBoxWhileBeingAWinner () $ this-> setAPlayerThatIsInThePenaltyBox (); $ this-> game-> add ('Un autre joueur'); $ this-> currentPlayerWillLeavePenaltyBox (); $ this-> setCurrentPlayerAWinner (); $ this-> assertTrue ($ this-> game-> wasCorrectlyAnswered ()); En ajoutant simplement un autre joueur dans chacun de nos tests liés à cette méthode, nous pouvons nous assurer que la logique est couverte. Ce test fera échouer l'instruction de retour modifiée ci-dessus.
fonction privée selectNextPlayer () $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Nous pouvons immédiatement remarquer que la sélection du joueur suivant est identique sur les deux chemins de la condition. Nous pouvons le déplacer dans une méthode à part. Le nom que nous avons choisi pour cette méthode est selectNextPlayer (). Ce nom aide à souligner le fait que la valeur du joueur actuel sera modifiée. Il suggère également que didPlayerNotWin () pourrait être renommé en quelque chose qui reflète la relation avec le joueur actuel.
function wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) if ($ this-> isGettingOutOfPenaltyBox) $ this-> display-> correctAnswer (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); retourner $ notAWinner; else $ this-> selectNextPlayer (); retourne vrai; else $ this-> display-> correctAnswerWithTypo (); $ this-> purses [$ this-> currentPlayer] ++; $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); retourner $ notAWinner;
Notre code devient plus court et plus expressif. Que pourrions-nous faire ensuite? Nous pourrions changer le nom étrange de la logique "pas gagnant" et changer la méthode en une logique positive au lieu d'une logique négative. Ou nous pourrions continuer à extraire et traiter la confusion de logique négative plus tard. Je ne pense pas qu'il y ait une solution définitive. Donc, je vais laisser le problème de la logique négative comme exercice pour vous et nous allons continuer avec l'extraction.
function wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); else return $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
En règle générale, essayez d’avoir une seule ligne de code sur chaque chemin d’une logique de décision..
Nous avons extrait tout le bloc de code dans chaque partie de notre si déclaration. C’est une étape importante à laquelle vous devriez toujours penser. Lorsque vous avez un chemin de décision ou une boucle dans votre code, l'intérieur ne devrait contenir qu'une seule instruction. La personne qui lit cette méthode ne s’intéressera probablement pas aux détails de la mise en oeuvre. Il se souciera de la logique de décision, de la si déclaration.
function wasCorrectlyAnswered () if ($ this-> inPenaltyBox [$ this-> currentPlayer]) return $ this-> getCorrectlyAnsweredForPlayersInPenaltyBox (); return $ this-> getCorrectlyAnsweredForPlayersNotInPenaltyBox ();
Et si nous pouvons nous débarrasser de tout code supplémentaire, nous devrions le faire. Retrait du autre et tout en maintenant la logique, nous avons fait un peu d’économie. J'aime mieux cette solution car elle met en évidence le comportement "par défaut" de la fonction. Le code qui se trouve directement dans l'intérieur de la fonction (la dernière ligne de code ici). le si instruction est la fonctionnalité exceptionnelle ajoutée à la valeur par défaut.
J'ai entendu des raisonnements selon lesquels écrire les conditions de cette façon peut masquer le fait que la défaillance ne s'exécute pas si si déclaration active. Je ne peux qu'être d'accord avec cela, et si vous préférez garder le autre partie là pour plus de clarté, s'il vous plaît faites-le.
fonction privée getCorrectlyAnsweredForPlayersInPenaltyBox () if ($ this-> isGettingOutOfPenaltyBox) return $ this-> getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox (); else return $ this-> getCorrectlyAnsweredForPlayerStayingInPenaltyBox ();
Nous pouvons continuer à extraire à l'intérieur de nos méthodes privées nouvellement créées. Appliquer le même principe à notre prochaine instruction conditionnelle conduit au code ci-dessus.
fonction privée giveCurrentUserACoin () $ this-> purses [$ this-> currentPlayer] ++;
En regardant nos méthodes privées getCorrectlyAnsweredForPlayersNotInPenaltyBox () et getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () nous pouvons immédiatement observer qu'une simple affectation est dupliquée. Cette cession peut être évidente pour quelqu'un comme nous, qui sait déjà ce qui se passe avec les bourses et les pièces de monnaie, mais pas pour un nouveau venu. Extraire cette seule ligne dans une méthode giveCurrentUserACoin () est une bonne idée.
Cela aide aussi avec la duplication. Si, à l'avenir, nous modifions la manière dont nous donnons des pièces aux joueurs, nous devrons modifier le code uniquement dans cette méthode privée..
fonction privée getCorrectlyAnsweredForPlayersNotInPenaltyBox () $ this-> display-> correctAnswerWithTypo (); return $ this-> getCorrectlyAnsweredForAPlayer (); fonction privée getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox () $ this-> display-> correctAnswer (); return $ this-> getCorrectlyAnsweredForAPlayer (); fonction privée getCorrectlyAnsweredForAPlayer () $ this-> giveCurrentUserACoin (); $ this-> display-> playerCoins ($ this-> players [$ this-> currentPlayer], $ this-> purses [$ this-> currentPlayer]); $ notAWinner = $ this-> didCurrentPlayerNotWin (); $ this-> selectNextPlayer (); retourner $ notAWinner;
Ensuite, les deux méthodes correctement répondues sont identiques, sauf que l’une d’elles génère quelque chose avec une faute de frappe. Nous avons extrait le code en double et gardé les différences dans chaque méthode. Vous pensez peut-être que nous aurions pu utiliser la méthode extraite avec un paramètre dans le code de l'appelant et sortir une fois normalement et une fois avec une faute de frappe. Cependant, la solution proposée ci-dessus présente un avantage: elle sépare les deux concepts de non entrée dans la surface de réparation et de sortie de la surface de réparation..
Ceci conclut les travaux sur wasCorrectlyAnswered ().
function wrongAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0; return true;
A 11 lignes, cette méthode n’est pas énorme, mais certainement grande. Vous souvenez-vous du nombre magique sept plus moins deux? Il est dit que notre cerveau peut simultanément penser à 7 + -2 choses. C'est-à-dire que nous avons une capacité limitée. Ainsi, afin de comprendre facilement et complètement une méthode, nous voulons que la logique qu’elle contient s’adapte à cette gamme. Avec un total de 11 lignes et un contenu de 9 lignes, cette méthode est un peu à la limite. Vous pouvez faire valoir qu'il existe en réalité une ligne vide et une autre avec juste une accolade. Cela ferait 7 lignes de logique seulement.
Bien que les accolades et les espaces soient courts, ils ont une signification pour nous. Ils séparent des parties de la logique, ils ont une signification, donc notre cerveau doit les traiter. Oui, c’est plus facile comparé à une ligne complète de logique de comparaison, mais.
C'est pourquoi notre objectif pour les lignes de logique dans une méthode est de 4 lignes. C’est en deçà du minimum de la théorie ci-dessus, de sorte qu’un programmeur de génie et médiocre devrait pouvoir comprendre la méthode..
$ this-> currentPlayer ++; if ($ this-> shouldResetCurrentPlayer ()) $ this-> currentPlayer = 0;
Nous avons déjà une méthode pour ce morceau de code, nous devrions donc l'utiliser.
function wrongAnswer () $ this-> display-> incorrectAnswer (); $ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> selectNextPlayer (); retourne vrai;
Mieux, mais devrions-nous laisser tomber ou continuer?
$ currentPlayer = $ this-> players [$ this-> currentPlayer]; $ this-> display-> playerSentToPenaltyBox ($ currentPlayer);
Nous pourrions en ligne la variable de ces deux lignes. $ this-> currentPlayer est évidemment de retourner le joueur actuel, donc pas besoin de répéter la logique. Nous n’apprenons rien de nouveau ni rien d’abstrait en utilisant la variable locale.
function wrongAnswer () $ this-> display-> incorrectAnswer (); $ this-> display-> playerSentToPenaltyBox ($ this-> players [$ this-> currentPlayer]); $ this-> inPenaltyBox [$ this-> currentPlayer] = true; $ this-> selectNextPlayer (); retourne vrai;
Nous sommes à 5 lignes. Quelque chose d'autre là?
$ this-> inPenaltyBox [$ this-> currentPlayer] = true;
Nous pouvons extraire la ligne ci-dessus dans sa propre méthode. Cela aidera à expliquer ce qui se passe et à isoler la logique voulant que le joueur actuel soit renvoyé dans le banc des punitions à sa place.
function wrongAnswer () $ this-> display-> incorrectAnswer (); $ this-> display-> playerSentToPenaltyBox ($ this-> players [$ this-> currentPlayer]); $ this-> sendCurrentPlayerToPenaltyBox (); $ this-> selectNextPlayer (); retourne vrai;
Encore 5 lignes, mais tous les appels de méthode. Les deux premiers affichent des choses. Les deux suivants sont liés à notre logique. La dernière ligne retourne simplement true. Je ne vois aucun moyen de rendre cette méthode plus facile à comprendre sans introduire de complexité par les extractions que nous pourrions réaliser, par exemple en extrayant les deux méthodes d'affichage en une méthode privée. Si nous voulions le faire, où cette méthode devrait-elle aller? Dans ce Jeu classe, ou dans Afficher? Je pense que cette question est déjà trop complexe pour mériter d’être examinée compte tenu de la simplicité même de notre méthode..
Faisons quelques statistiques en consultant ce formidable outil de l'auteur de PHPUnit https://github.com/sebastianbergmann/phploc.git
./ phploc… / Refactoring \ Legacy \ Code \ - \ Part \ 1 \: \ Le \ Golden \ Master / Source / trivia / php / phploc 2.1-gca70e70 de Sebastian Bergmann. Lignes de code de taille (LOC) 232 Lignes de code de commentaire (CLOC) 0 (0,00%) Lignes de code sans commentaire (NCLOC) 232 (100,00%) Lignes de code logiques (LLOC) 99 (42,67%) Classes 88 (88,89) %) Longueur moyenne de la classe 88 Longueur minimale de la classe 88 Longueur maximale de la classe 88 Longueur moyenne de la méthode 7 Longueur minimale de la méthode 1 Longueur maximale de la méthode 17 Fonctions 1 (1,01%) Durée moyenne de la fonction 1 Pas dans les classes ni dans les fonctions 10 (10,10%) Complexité cyclomatique Complexité moyenne par LLOC 0,26 Complexité moyenne par classe 25,00 Complexité de classe minimale 25,00 Complexité de classe maximale 25,00 Complexité moyenne par méthode 3.18 Complexité de la méthode minimale 1,00 Complexité de la méthode maximale 10,00 Dépendances Accès globaux 0 Constantes globales 0 (0,00%) Variables globales 0 (0,00%) Super-Global Variables 0 (0.00%) Accès aux attributs 115 Non statique 115 (100.00%) Statique 0 (0.00%) Appels de méthode 21 Non statique 21 (100.00%) Statique 0 (0.00%) Structure Espaces de nommage 0 Interfaces 0 Traits 0 Classes 1 Résumé Classes 0 (0.00%) Béton Classes 1 ( 100,00%) Méthodes 11 Domaine d'application Méthodes non statiques 11 (100,00%) Méthodes statiques 0 (0,00%) Visibilité Méthodes publiques 11 (100,00%) Méthodes non publiques 0 (0,00%) Fonctions 1 Fonctions nommées 1 (100,00%) Fonctions anonymes 0 (0,00%) constantes 0 constantes globales 0 (0,00%) constantes de classe 0 (0,00%)
./ phploc… / Refactoring \ Legacy \ Code \ - \ Part \ 11 \: \ The \ End \? / Source / trivia / php phploc 2.1-gca70e70 de Sebastian Bergmann. Lignes de code de taille (LOC) 371 Lignes de code de commentaire (CLOC) 0 (0,00%) Lignes de code sans commentaire (NCLOC) 371 (100,00%) Lignes de code logiques (LLOC) 151 (40,70%) Classes 145 (96,03%) %) Longueur moyenne de la classe 36 Longueur minimale de la classe 8 Longueur maximale de la classe 89 Longueur moyenne de la méthode 2 Longueur minimale de la méthode 1 Longueur maximale de la méthode 14 Fonctions 0 (0,00%) Durée moyenne de la fonction 0 Pas dans les classes ni dans les fonctions 6 (3,97%) Complexité cyclomatique Complexité moyenne par LLOC 0,15 Complexité moyenne par classe 6.50 Complexité de classe minimale 1,00 Complexité de classe maximale 17,00 Complexité moyenne par méthode 1,46 Complexité minimale de la méthode 1,00 Complexité de la méthode maximale 10,00 Dépendances Accès globaux 0 Constantes globales 0 (0,00%) Variables globales 0 (0,00%) Super-global Variables 0 (0.00%) Accès aux attributs 96 Non statiques 94 (97.92%) Statiques 2 (2.08%) Méthode Appels 74 Non statiques 74 (100.00%) Statiques 0 (0.00%) Structure Espaces de nommage 0 Interfaces 1 Traits 0 Classes 3 Résumé Classes 0 (0,00%) Béton Classes 3 (100,00 %) Méthodes 59 Champ d'application Méthodes non statiques 59 (100,00%) Méthodes statiques 0 (0.00%) Visibilité Méthodes publiques 35 (59.32%) Méthodes non publiques 24 (40.68%) Fonctions 0 Fonctions nommées 0 (0.00%) Fonctions anonymes 0 (0,00%) Constantes 3 Constantes globales 0 (0,00%) Constantes de classe 3 (100,00%)
Les données brutes sont aussi bonnes que nous pouvons les comprendre et les analyser.
Le nombre de lignes de code logiques a considérablement augmenté, passant de 99 à 151. Ce nombre ne doit toutefois pas vous faire croire que notre code est devenu plus complexe. C’est une tendance naturelle du code bien refactorisé, en raison de l’augmentation du nombre de méthodes et d’appels à ces méthodes..
Dès que nous examinons la longueur moyenne des classes, nous constatons une chute spectaculaire du nombre de lignes de code, qui passe de 88 à 36..
Et il est simplement étonnant de voir comment la longueur de la méthode est passée d’une moyenne de sept lignes à seulement deux lignes de code..
Bien que le nombre de lignes soit un bon indicateur du volume de code par unité de mesure, le réel avantage réside dans les analyses de la complexité cyclomatique. Chaque fois que nous prenons une décision dans notre code, nous augmentons la complexité cyclomatique. Quand on enchaîne si Dans les deux déclarations, la complexité cyclomatique de cette méthode augmente de façon exponentielle. Nos extractions continues ont abouti à des méthodes avec une seule décision, ramenant ainsi leur complexité moyenne par méthode de 3,18 à 1,00. Vous pouvez lire ceci comme "nos méthodes refactorisées sont 3,18 fois plus simples que le code d'origine". Au niveau de la classe, la diminution de la complexité est encore plus étonnante. Il est descendu de 25h à 6h50.
Bien. C'est tout. Fin de la série. N'hésitez pas à exprimer vos opinions. Si vous pensez que nous avons oublié un sujet de refactoring, demandez-le dans les commentaires ci-dessous. S'ils sont intéressants, je les transformerai en parties supplémentaires de cette série..
Merci pour votre attention sans partage.
