Intervenir sur des bases de code pénibles constitue une des réalités ingrates du métier de développeur.

Ce type de code est fréquemment qualifié du terme politiquement correct de legacy. A l’origine, on désignait ainsi du code très ancien, et peu voire pas documenté. Le consensus actuel se réfère à du code démuni de tests, peu importe qu’il date ou qu’il sorte encore fumant du clavier.

Lorsqu’on est confronté à un tel code, on sait qu’on va passer un moment pénible, avec les difficultés suivantes :

• Le code en lui-même est difficile à comprendre
• L’absence de tests
• Rend le besoin auquel le code répond plus difficile à cerner
• Empêche de travailler en sécurité, nous exposant à des régressions

On peut néanmoins se faciliter la tâche (ou a minima la rendre moins éprouvante) au moyen de certains outils et techniques. Pour cela, nous nous appuierons, à travers une série d’articles, sur des kata  de code conçus spécifiquement pour aborder ces problématiques.

 

Un kata de code consiste en un exercice de programmation permettant aux développeurs de se perfectionner par la pratique et la répétition.

Les kata de refactoring ont la particularité de proposer une base de code existante, l’objectif étant de retravailler le code. Le code est rendu délibérément déplaisant et incompréhensible.

Le premier kata de refactoring que nous allons aborder est le kata Gilded Rose.

Le kata Gilded Rose

Dans le kata Gilded Rose, le code est rendu obscur par une complexité cyclomatique importance (beaucoup de chemins possibles), et les tests se distinguent par leur absence. Toute ressemblance avec du code réellement en production serait bien évidemment purement fortuite.

T’façon ce qui compte, c’est les valeurs

Intéressons nous maintenant au problème posé par Gilded Rose (trouvable sur le Github d'Emily Bache), décrit dans le document d'expression des besoins.

On nous parle d’une petite auberge dans une cité prospère, qui propose des articles à la vente. Seulement, leur qualité se dégrade au fur et à mesure que leur date limite de vente approche. Pour se faciliter la gestion d’inventaire, l’aubergiste, Allison, a demandé à Leeroy, un aventurier, de développer une application pour ça.

Ce dernier étant parti pour de nouvelles aventures, elle nous demande d’intervenir pour supporter une nouvelle catégorie d’articles (de type “Conjured”), dont la qualité se dégrade deux fois plus rapidement que d’ordinaire.

Chaque article est caractérisé par un nom, une qualité qui indique sa valeur intrinsèque, et une date limite désignant le nombre de jours avant lequel l’article doit être vendu.

Dès qu’on discute des règles de gestions, les choses se corsent. On se retrouve face à de nombreux cas particuliers, et on a du mal à distinguer le général du spécifique. A l’issue de la discussion on ne se sent pas très avancé.

Heureusement, on peut essayer de s’appuyer sur le code pour avancer dans la compréhension du problème. Ou pas :

A la lecture du code, on ne retrouve pas vraiment les règles métiers énoncées par l’aubergiste. L’ensemble est peu clair, avec plusieurs niveaux de branchements imbriqués et de nombreux effets de bord (en violant allègrement plusieurs principes et bonnes pratiques du clean code). Les choses se présentent donc plutôt mal …

 

Remarque: le code présenté ici correspond à la version Java du kata. Si on est plus familier avec d’autres langages, ce n’est pas un problème : le kata est disponible dans de nombreux langages.

Man versus wild legacy

La première difficulté consiste à savoir par où commencer. Étudions les options qui s’offrent à nous.

Attaquer bille en tête

L’évolution demandée ne semblant pas très compliquée, on pourrait l’implémenter directement dans le code.

Cette méthode est très aléatoire tant on ne dispose d’aucune garantie de répondre au besoin tout en préservant le comportement existant. Cette approche de tête brûlée est donc à proscrire ...

Prendre le temps de comprendre

A l’inverse, on peut se poser, se munir de papier et essayer de comprendre le code. Seulement, ça peut être très chronophage et mener à des interprétations erronées du comportement de l’application.

Ecrire des tests

Modifier le code tel quel reviendrait à bâtir sur des ruines. Nous allons plutôt choisir de prednre soin de nous et de mettre en place des tests. Ils nous permettront d’apporter des changements en limitant les risques, tout en améliorant notre compréhension du code. C’est donc le premier objectif vers lequel on doit tendre.

Hé ! Pas si vite …

Avant cela, il faut s’assurer que le code est dans un état qui permet d’écrire des tests. On doit donc être capable d’exécuter le code. Pour cela, on doit disposer de tous les éléments nécessaires (code et dépendances), et configurer le projet correctement.

Le kata Gilded Rose se présentant sous la forme d’un projet Maven, il est facile de l’importer et de le compiler dans son IDE préféré. On trouve même une classe exécutable (avec une méthode main()) qui appelle quelques fonctions du code et affiche le résultat dans la sortie standard.

On peut jeter un œil sur le code de production également. Le kata fournit deux classes :

• Item : décrit les articles mis en dépôt à l’auberge, caractérisés par un nom, une qualité et une date limite
• GildedRose : gère une collection d’articles et possède une unique méthode, updateQuality() pour gérer le cycle de vie des articles ; c’est au niveau de cette méthode que nous sommes supposés intervenir

Ecriture des tests

Le but de nos tests est de garantir l’absence de régression, tout en couvrant le plus possible de code.

Le premier test

Dans une situation classique, nous partons des besoins, et construisons les tests de façon incrémentale (en baby steps), en suivant l’approche TDD. On écrit un premier test, assez simple, puis une fois qu’il fonctionne, on écrit un deuxième, un peu plus complexe, et ainsi de suite ...

Mais dans la situation présente, nous avons déjà affaire à un existant.

Nous disposons certes d’un document d’expression des besoins. Mais, nous l’avons constaté, la discussion avec l’aubergiste était compliquée, les concepts métier sont mélangés, on distingue mal le général du spécifique. On pourrait retourner voir Allison, mais la journée avançant (et les boissons partagées avec les clients aussi), on ne peut compter sur une description plus claire des besoins.

De façon nettement plus problématique, rien ne garantit que l’implémentation actuelle soit conforme aux besoins. De façon assez naturelle, l’implémentation s’écarte assez rapidement de la documentation, et au final, c’est dans le code que réside la vérité. D’un certain point de vue, le code est la documentation la plus importante d’un système d’information (cf. Code As Documentation).

 

Partir du code constitue une solution viable dans certaines situations de refactoring. Il faut néanmoins que l’ensemble ne soit pas trop complexe, permettant de voir un point de départ assez rapidement. Avec Gilded Rose, c’est loin d’être immédiat.

Le métier, c’est pas si important … au début

La vrai difficulté provient du fait que, de façon assez intuitive, on cherche à se raccrocher au métier et à le comprendre pour être guidé. La complexité intrinsèque de Gilded Rose rend cette approche contre-productive.

Pour s’en sortir, il est nécessaire d’oublier, pour un temps au moins, le métier. Nous allons chercher à couvrir le maximum de code en un minimum d’efforts, de façon brute. Nous allons modifier la perspective des tests : on ne cherche plus à vérifier ce que le code fait, mais plutôt qu’il fait exactement la même chose qu’avant.

Pour cela, nous allons construire un golden master, remplissant le rôle d’un harnais de sécurité.

Confection d’un Golden Master

Principes

Un golden master peut se résumer comme l’ensemble des comportements qui caractérisent l’existant (on parle aussi de tests de caractérisation).

La première étape consiste à produire les résultats de tests de référence pour le code existant. On pourra ensuite les comparer aux résultats de ces mêmes tests pour le code en cours de refactoring.

Le comportement du code à tester étant plutôt obscur, le scénario des tests ne peut ni se baser sur les fonctionnalités, ni vérifier des assertions liées au métier (comme on le ferait habituellement avec du TDD ou du BDD).

L’idée est plutôt de produire un ensemble d’inputs de façon brute, afin de couvrir un maximum de chemins possibles dans le code, et d’en collecter les outputs.

De tels tests auront donc une valeur documentaire quasi-nulle, leur intérêt étant avant tout d’assurer la non-régression. L’idéal est de les considérer de façon éphémère, le temps d’assurer la réécriture du code (on peut voir ça comme un échafaudage qui nous permettrait de nous en sortir le temps de tout mettre au propre).

Construction

Il existe différentes façons d’effectuer des tests en golden master.

La première d’entre elles consiste à générer des données de référence (textes, images, données binaires) correspondant à ce qui est habituellement produit par le code legacy. Ces données seront ensuite comparées avec la sortie du code réécrit.

Par exemple on comparera un fichier de texte de référence avec le texte produit dans la sortie standard. On peut y voir un parallèle avec le approval testing, l’idée étant de comparer des échantillons textuels correspondant à des objets complexes plutôt que d’écrire des multitudes d’assertions.

Les échantillons textuels peuvent être fournis directement dans le code des tests, si la volumétrie le permet. Dans le cas de données massives, on sera forcé de les fournir dans des fichiers, avec tout ce que ça implique pour outiller les tests.

Pour Gilded Rose, nous allons essayer de fournir les données directement dans le code du test.

Application à Gilded Rose

Les inputs

La définition des inputs peut constituer une difficulté en soi : il faut s’assurer de proposer des données en entrée suffisamment représentatives de ce qui peut être accepté (ou non, pour les cas d’erreurs) par l’application. Les entrées doivent être assez exhaustives pour déclencher une couverture de code suffisante.

 

Par chance, le kata Gilded Rose fournit une collection d’Item prête à l’emploi :

Item[] items = new Item[]{
       new Item("+5 Dexterity Vest", 10, 20),
       new Item("Aged Brie", 2, 0),
       new Item("Elixir of the Mongoose", 5, 7),
       new Item("Sulfuras, Hand of Ragnaros", 0, 80),
       new Item("Sulfuras, Hand of Ragnaros", -1, 80),
       new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20),
       new Item("Backstage passes to a TAFKAL80ETC concert", 10, 49),
       new Item("Backstage passes to a TAFKAL80ETC concert", 5, 49),
       // this conjured item does not work properly yet
       new Item("Conjured Mana Cake", 3, 6)
};

 

Il suffit d’initialiser un objet GildedRose avec cette collection d’articles dans les tests, et le tour est joué.

Et là, on pourrait légitimement se demander si le kata n’essaye pas un peu de nous enfumer. Un tout petit peu en fait. Dans une situation réelle, les entrées de test ont peu de chances d’être servies sur un plateau.

C’est une situation dont il faut tenir compte. Ici malheureusement, il faut expérimenter, tatonner et construire le jeu de tests de façon laborieuse. D’autres kata de refactoring (comme le TripService de Sandro Mancuso) mettent en avant des techniques pour se faciliter la vie, mais ce n’est pas l’objet du kata auquel nous nous intéressons en ce moment.

La méthode

Nous allons nous appuyer sur le point de vue le code de production a systématiquement raison. Pour construire nos tests, nous allons donc récupérer les sorties obtenues et nous en servir comme valeurs de référence : dans les assertions, elles définiront les valeurs attendues. On met une valeur bidon (null ou collection vide) en attendu, et on laisse le test échouer. Le framework de test, dans sa grande bonté, va nous indiquer la valeur obtenue. Il suffit de la recopier en tant que valeur attendue dans l’assertion, et le tour est joué.

On procède alors de façon incrémentale, à chaque fois que les tests sont au vert, on complexifie un peu plus les actions pour le faire échouer, on recopie la valeur obtenue pour le faire fonctionner, et on recommence.

Pour GildedRose, on vérifiera pour chacun de nos tests l’état de chaque article déposé à l’auberge. On commence simplement, en vérifiant déjà l’état à l’initialisation. Ce premier test peut paraître digne du Captain Obvious, il permet malgré tout de s’assurer que la comparaison se fait correctement dans les assertions.

On échoue avec un tir à blanc :

 

 

 

On fait passer le test :

 

 

 

 

 

Ensuite on va vérifier le comportement de la fonctionnalité qui nous intéresse, à savoir la méthode updateQuality(). En l’exécutant une première fois. Puis 2 fois d’affilée (les résultats sont différents). Et ainsi de suite.

L’approche est assez naïve, mais elle nous permet de démarrer sans trop se faire de noeuds au cerveau. La difficulté va être de savoir quand s’arrêter.

Refactorez couverts

Pour se fixer une limite dans l’effort, on va s’appuyer sur la couverture de code. Le principe est simple : on utilise un outil automatique qui, lors de l’exécution des tests, va marquer le code de production de la façon suivante :

• les lignes de code exécutées au moins une fois par les tests apparaissent en vert
• les autres lignes apparaissent en rouge

 

Le résultat obtenu avec un seule exécution de la méthode updateQuality() est le suivant :

On peut imaginer que la couverture va augmenter avec le nombre d’exécutions successives de updateQuality(). Une fois toutes les lignes couvertes, on pourra se dire qu’on a exécuté le code suffisamment de fois.

Avant de trouver le nombre correct d’exécutions, il peut être fastidieux de modifier les valeurs attendues à chaque essai.

On n’est pas obligé de s’infliger ça, en profitant d’une propriété importante du code coverage : les lignes parcourues sont marquées quel que soit le résultat des assertions (succès ou échec). On va donc d’abord chercher à couvrir totalement le code, sans se préoccuper des assertions, pour trouver le nombre suffisant d’exécutions. Ceci atteint, on pourra se concentrer sur les assertions.

Remarque : cette propriété de la couverture de code est toutefois à double-tranchant, dans la mesure où il n’y a aucune garantie sur la pertinence des assertions. Nous reviendrons sur ce point un peu plus loin.

 

Il se trouve que 16 exécutions de updateQuality() suffisent à atteindre une couverture complète.

Remarque : on cherche à atteindre 100% de couverture lorsque c’est possible sans nécessiter des efforts surhumains, sinon on cherche juste à atteindre le meilleur score. De la même manière, on se concentre seulement sur le périmètre de code à réécrire, il n’est pas nécessaire de couvrir tout le projet. L’idée est de rester économe voire fainéant dans la démarche.

Moins et moins ça fait plus

Nos tests sont-ils suffisamment fiables ? Actuellement, seul l’état final (au bout des n itérations) de l’application est vérifié. Ce n’est pas le cas des états intermédiaires.

Pour bien faire, il faudrait tester l’état après chaque itération.

Nous ne sommes en effet pas à l’abri de provoquer des régressions locales, qui au niveau global s’annuleraient, et passeraient ainsi inaperçues.

Ceci nous impose d’écrire les assertions pour chaque itération. Il faudrait définir dans le code de test les valeurs de chaque état. Ca risque de prendre des plombes …

Notre approche naïve montre ici ses limites, nous obligeant à réfléchir un peu. Faisons preuve d’audace en changeant de point de vue.

On s’est concentré sur des valeurs spécifiques. Mais dans l’absolu, on s’en moque. D’autant qu’elles n’ont aucun rôle documentaire.

 

Revenons sur ce qui est important : s’assurer que la nouvelle version du code a un comportement identique à la version legacy. Il suffit simplement de s’assurer que les valeurs produites sont identiques entre les deux versions.

Pour cela, on duplique l’implémentation legacy, pour se retrouver avec deux implémentations :

• GildedRose : cette implémentation va être réécrite
• LegacyGildedRose : c’est notre implémentation de référence, elle ne doit jamais être modifiée (remarque : on peut même déplacer cette implémentation dans l’espace de code dédié aux tests)

 

Ce n’est pas très intuitif, et pourtant ça fonctionne. Dans le test, il ne reste plus qu’à exécuter les implémentations en parallèles et à comparer les valeurs produites à chaque étape :

@Test
public void should_refactored_behave_exactly_as_legacy() {
   GildedRose inn = new GildedRose(items);
   LegacyGildedRose legacyInn = new LegacyGildedRose(legacyItems);

   for (int i = 0; i < 16; i++) {
       inn.updateQuality();
       legacyInn.updateQuality();

       assertThat(inn.items).extracting("name").containsExactly(
   Arrays.stream(legacyInn.items).map(item -> item.name).toArray());
       assertThat(inn.items).extracting("quality").containsExactly(
 Arrays.stream(legacyInn.items).map(item -> item.quality).toArray());
       assertThat(inn.items).extracting("sellIn").containsExactly(
  Arrays.stream(legacyInn.items).map(item -> item.sellIn).toArray());
   }
}

 

Il faut s’assurer que chaque implémentation travaille sur des instances différentes d’Items. Pour cela, on fait une copie en profondeur :

Item[] items = new Item[]{
       new Item("+5 Dexterity Vest", 10, 20), 
       new Item("Aged Brie", 2, 0), 
       new Item("Elixir of the Mongoose", 5, 7), 
       new Item("Sulfuras, Hand of Ragnaros", 0, 80), 
       new Item("Sulfuras, Hand of Ragnaros", -1, 80),
       new Item("Backstage passes to a TAFKAL80ETC concert", 15, 20),
       new Item("Backstage passes to a TAFKAL80ETC concert", 10, 49),
       new Item("Backstage passes to a TAFKAL80ETC concert", 5, 49),
       // this conjured item does not work properly yet
       new Item("Conjured Mana Cake", 3, 6)};

Item[] legacyItems = Arrays.stream(items)
       .map(item -> new Item(item.name, item.sellIn, item.quality))
       .toArray(Item[]::new);

Nous disposons enfin de notre golden master de test.

Au départ, tous les tests doivent être verts, étant donné que les deux implémentations sont identiques. Si ce n’est pas le cas, c’est qu’on a un problème dans l’écriture des tests (une erreur classique est de comparer les références des objets Item dans les assertions plutôt que leur contenu).

Golden master will remember that

Pour construire notre golden master, nous nous sommes finalement éloigné de l’approche classique en Text-based approval-testing. Cette approche, bien qu’intuitive, peut se montrer laborieuse avec des volumétries importante de données.

En optant pour une exécution en parallèle, notre vie est devenue moins compliquée. Mais en faisant ça, nous avons modifié l’essence de notre golden master : ce n’est plus l’ensemble de résultats, comme dans l’approche classique, mais directement la version legacy du code. La conséquence directe est qu’à l’avenir, nous aurons quelques difficultés à éliminer complètement cette version legacy du projet : nous sommes forcés de la conserver pour exécuter nos tests. Mais nous aurons l’occasion de revenir sur ce point plus tard.

Il ne peut plus rien nous arriver de mal ?

Les tests passent et le code est couvert à 100%. Est-ce suffisant pour se lancer dans le refactoring en toute sécurité ?

Le succès des tests nous garantit que l’exécution du code de production n’a rencontré aucune erreur ou invalidé aucune assertions. Le degré de confiance dépend alors de la pertinence et de l’exhaustivité des assertions.

Les 100% de couverture nous assurent que toutes les lignes du code de production ont été exécutées lors des tests, guère plus. Si dans le test actuel on commente les assertions, il sera quand même vert, avec une couverture à 100%, alors qu’en fait rien n’est testé.

De plus, en couverture de code, seul le parcours d’une ligne est vérifié, mais pas son contenu ou son comportement.

Ce constat n’est pas très rassurant, et nous amène à nous dire que peu importe les efforts fournis, on ne disposera d’aucune garantie de sécurité absolue. Mais est-ce si grave ?

C’est déjà moins grave que la situation de départ : absence de tests et code incompréhensible. Maintenant qu’on a des tests, on peut commencer à faire quelque chose. C’est mieux que de réécrire sans aucun test, sans filet. Il faut juste être prêt à accepter une part de risque résiduel.

 

Ensuite, il existe des techniques additionnelles de test permettant de réduire encore davantage ce risque, comme le mutation testing.

Pour faire simple, le mutation testing consiste à modifier dynamiquement le code de production lors de l’exécution des tests, et s’assurer que ces modifications font échouer au moins un test. Dans le cas contraire, on est généralement face à une des causes suivantes :

• les assertions ne sont pas suffisamment exhaustives
• la ligne de code qui a été mutée ne sert pas à grand-chose

Il se trouve que sur le kata Gilded Rose, le mutation testing ne montre pas de problèmes particuliers. Le kata a en effet été calibré pour qu’une couverture de code à 100% soit suffisante pour refactorer en sécurité. Nous ne rentrerons donc pas plus dans les détails du mutation testing dans l’immédiat.

Et après ?

Maintenant qu’on dispose d’un filet de sécurité, on peut refactorer le code en toute quiétude. Cette phase de réécriture du code fera l’objet d’un second article.

 

GitHub : https://github.com/athiefaine/gilded-rose-kata

Rétrospective

Faisons le bilan de ce que nous a appris cette première partie de kata :

• ne pas se résigner ni se désespérer devant un code pourri, on peut s’en sortir en affrontant un minimum de difficultés
• prendre de la hauteur pour tenter des approches contre-intuitives
  • construire un golden master, autrement dit écrire des tests de caractérisation basés autour des résultats de référence, plutôt que des tests basés sur la compréhension de l’application
  • dupliquer les implémentations pour les comparer lors des tests
• faire confiance à la couverture de tests, mais pas aveuglément, en prenant conscience de ses limites
• faire attention à la pertinence des assertions
• s’appuyer sur les outils et méthodologies, pour libérer le maximum de temps de cerveau

Remerciements

Je tiens à remercier les personnes suivantes pour le temps passé à relire mon texte, et les différents conseils, idées et encouragements qu’ils ont pu me prodiguer : Dorra Bartaguiz, Alexia Hoang, Benjamin Hugot, Arnauld Loyer, Cyrille Martraire, Laury Maurice, Hadrien Mens-Pellen, Nicolas Morin et Yvan Phélizot.