Refactoring: Corrigez vos bugs avant qu’ils ne passent production

Cela fait du bien de corriger un vilain bug, n’est-ce pas? Mais c’est toujours une expérience douloureuse quand nos utilisateurs en découvrent de nouveaux. Alors pourquoi ne pas les résoudre «par la conception», avant même qu’ils ne passent en production?

Une nouvelle histoire

Il n’y a pas si longtemps, dans une base de code très dépassée…

Cette introduction nous semble familière, n’est-ce pas ? Dans le cycle de vie d’une application, il est plus que fréquent d’ajouter des fonctionnalités sur une base de code hérité. La plupart du temps, le programmeur de ce code n’avait tout simplement pas prévu qu’une telle fonctionnalité soit demandée.

Pour cette raison, chaque nouvelle fonctionnalité devrait être l’occasion de remettre en question la conception de cette base de code. Il y a deux avantages à une meilleure conception qui pourraient satisfaire à la fois le Product Owner / Client et l’équipe de développement:

• Une conception adaptée peut permettre à la fonctionnalité d’être implémentée plus rapidement
• Une bonne conception peut générer moins de bugs sur le long terme

Le pire code attaque

En parlant de bugs, ils se produisent généralement quand l’application est utilisée de manière non envisagée nous autres développeurs(euses). Cela signifie que nous ne pouvons pas vraiment les corriger avant que l’utilisateur ne les trouve, à moins que …

… nous puissions les éviter (pour des cas spécifiques)! Les programmeurs expérimentés ont remarqué des anti-modèles de conception qui ont été répétés dans de nombreux programmes; anti-modèles qui conduisent habituellement à des bugs d’une manière plus moins attendue. Une grande partie de ces anti-modèles ont été compilés par Martin Fowler et Kent Beck dans un livre nommé Refactoring: Comment améliorer le code existant¹, sous l’appellation de « code smells« .

L’exemple suivant vient du Bug Zero Kata² de Johan Martinsson. Ce code kata vise à prévenir les bugs se produisant avec la mise en œuvre de nouvelles fonctionnalités, en refactorisant une application proche d’un jeu de Trivial Pursuit.

function add($playerName) {
   array_push($this->players, $playerName);
   $this->places[$this->howManyPlayers()] = 0;
   $this->purses[$this->howManyPlayers()] = 0;
   $this->inPenaltyBox[$this->howManyPlayers()] = false;

    echoln($playerName . " was added");
    echoln("They are player number " . count($this->players));
	return true;
}

Les lignes ci-dessus (en PHP) montrent un exemple d’Amas de données. Il s’agit d’un code smell typique: vous pouvez voir que, chaque fois qu’un nouveau joueur entre dans le jeu, quatre valeurs sont ajoutées dans quatre tableaux différents:

• players : le nom du nouveau joueur, passé comme paramètre.
• places : la position du nouveau joueur sur le plateau, 0 par défaut.
• purses : la quantité de pièces collectées du nouveau joueur (lorsqu’il répond correctement à une question), également 0 par défaut.
• inPenaltyBox : si un nouveau joueur est dans la case « prison » (quand il répond incorrectement à une question), false par défaut.

Bien que toutes ces valeurs soient liées à une même joueuse, elles sont réparties sur quatre tableaux différents ! Nous pouvons facilement voir comment cela peut causer un bug lors d’un futur développement: imaginez qu’une joueuse soit retirée de la liste joueurs par une nouvelle fonction comme celle-ci:

function remove($playerNumber) {
    $playerName = $this->players[$playerNumber];
    array_splice($this->players, $playerNumber, 1);

    echoln( $playerName . " leaved");
    for( $i = 0; $i < $this->howManyPlayers(); $i++ ) {
        echoln($this->players[$i] . " is now number " . $i );
    }
}

Vous pouvez voir le développeur a oublié de supprimer également les valeurs correspondant à cette joueuse pour places, purses et inPenaltyBox… Que se passe-t-il ici si nous supprimons le deuxième joueur dans un jeu de quatre joueurs? Eh bien, la troisième joueuse aura maintenant la position du deuxième joueur précédent, ses pièces et sa place sur la case prison. Le quatrième joueur aura les valeurs de la troisième joueuse précédente et ainsi de suite! Évidemment, cela satisfera plus certains joueurs que d’autres…

Le retour du code propre

Heureusement, il n’y a pas que les code smells qui sont bien connus, mais aussi les méthodes pour les contrer: les refactorings! Bien que le mot ait été utilisé pour définir toute modification du code qui n’ajoute pas de fonctionnalité à une base de code, il est utilisé dans le livre précédemment mentionné avec une signification très spécifique :

Refactoring (nom): un changement apporté à la structure interne du logiciel pour le rendre plus facile à comprendre et moins cher à modifier sans changer son comportement observable.

Martin Fowler – Refactoring: Comment améliorer le code existant

Ce livre contient un large catalogue de ces refactorings, qui sont comme des «recettes» à appliquer étape par étape. Et chacun d’eux peut être composé de plusieurs plus petits refactorings.

Prenons l’amas de données « reniflé » plus tôt comme exemple. Le livre recommande un refactoring nommé Extraire Classe: nous préférons avoir toutes les données connexes encapsulées dans un objet qui peut représenter un joueur. Donc, si un joueur quitte la partie, il prend toutes ses pièces avec lui.

Mais remplacer toutes les références aux variables places, purses et inPenaltyBox d’un seul coup serait très risqué. Nous allons donc d’abord faire un pas de plus, sous la forme du refactoring Encapsuler Champ :

public function getPursesForCurrentPlayer() {
    return $this->purses[$this->currentPlayer];
}

public function addPurseForCurrentPlayer() {
    $this->purses[$this->currentPlayer]++;
}

public function isCurrentPlayerInPenaltyBox() {
    return $this->inPenaltyBox[$this->currentPlayer];
}

public function setCurrentPlayerInPenaltyBox() {
    $this->inPenaltyBox[$this->currentPlayer] = true;
}

public function getPlaceForCurrentPlayer() {
    return $this->places[$this->currentPlayer];
}

public function movePlayerBy($roll) {
    $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $roll;
    if ($this->places[$this->currentPlayer] > 11) $this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] - 12;
}

Chacune de ces fonctions est un simple accesseur ou mutateur: ils encapsulent l’accès aux données de la joueuse actuelle (position sur le plateau, pièces de monnaie dans la bourse et si elle est dans la case « prison »), mais les tableaux existent toujours individuellement! Ce que je peux faire maintenant, c’est remplacer chaque appel à ces tableaux par un appel à la fonction adéquate à la place. Un par un, en testant entre chaque remplacement que je n’aie pas cassé quoi que ce soit. Avec un IDE intelligent ou un peu de « rechercher – remplacer », je peux vraiment accélérer le processus.

Maintenant que j’ai protégé l’accès aux données des joueurs, je peux créer la classe Player qui les représente:

class Player {
    private $name;
    private $place = 0;
    private $coins = 0;
    private $inPenaltyBox = false;

    public function __construct($playerName) {
        $this->name = $playerName;
    }

    public function getName() {
        return $this->name;
    }

    public function getPlace() {
        return $this->place;
    }

    public function moveBy($number) {
        $this->place = $this->place + $number;
        if ($this->place > 11) $this->place = $this->place - 12;
    }

    public function getCoins() {
        return $this->coins;
    }

    public function addCoin() {
        $this->coins++;
    }

    public function isInPenaltyBox() {
        return $this->inPenaltyBox;
    }

    public function setInPenaltyBox() {
        $this->inPenaltyBox = true;
    }
}

Parce que les valeurs de départ pour la position d’un nouveau joueur, ses pièces de monnaie et son placement (ou non) dans la case « prison » sont toujours les mêmes, je peux simplement les définir avec des valeurs par défaut dans la classe. Seul le nom du joueur doit être passé en paramètre dans le constructeur de classe. Le reste des méthodes de classe sont de simples accesseurs et mutateurs pour accéder à ces valeurs. Oui, cela fait beaucoup d’accesseurs et de mutateurs dans un même projet, mais je vais remplacer les premiers par ces derniers d’ici quelques minutes. Mais avant cela, j’ai besoin de changer la façon dont les données du joueur sont stockées.

function add($playerName) {
 	$player = new Player($playerName);
    array_push($this->players,$player);

    echoln($player->getName() . " was added");
    echoln("They are player number " . count($this->players));
	return true;
}

Cette fonction est déjà plus simple. Maintenant, le tableau players stocke des instances de la classe Player au lieu de simples chaînes de caractère représentant leurs noms. Cela signifie que je dois modifier le code suivant immédiatement pour afficher le nom du nouveau joueur en appelant l’accesseur Player::getName() au lieu d’accéder directement aux valeurs du tableau.

Mais attendez une minute! Puisque players stocke désormais des objets au lieu de chaînes de caractères, cela ne risque-t-il pas de casser toute mon application? La réponse est oui, mais aussi malin(e)s que vous êtes, vous comprenez sûrement où je veux en venir: tous les accès aux données ont été encapsulés dans des accesseurs et des mutateurs! Cela signifie que je n’ai qu’à changer le code à l’intérieur de ces accesseurs et mutateurs, et voilà, j’ai maintenant complètement changé la façon dont les données des joueuses sont stockées et accessibles dans toute mon application.

public function getNameForCurrentPlayer() {
    return $this->players[$this->currentPlayer]->getName();
}

public function getPursesForCurrentPlayer() {
    return $this->players[$this->currentPlayer]->getCoins();
}

public function addPurseForCurrentPlayer() {
    $this->players[$this->currentPlayer]->addCoin();
}

public function isCurrentPlayerInPenaltyBox() {
    return $this->players[$this->currentPlayer]->isInPenaltyBox();
}

public function setCurrentPlayerInPenaltyBox() {
    $this->players[$this->currentPlayer]->setInPenaltyBox();
}

public function getPlaceForCurrentPlayer() {
    return $this->players[$this->currentPlayer]->getPlace();
}

public function movePlayerBy($roll) {
    $this->players[$this->currentPlayer]->moveBy($roll);
}

J’aurais pu m’arrêter ici, mais ces accesseurs et mutateurs ne font que déléguer aux accesseurs et mutateurs des instances de Player… Cela fait un peu trop d’indirection à mon goût. J’ai donc fait un peu de nettoyage en:

• Extrayant l’accès à l’instance de la joueuse dont c’est le tour dans une méthode getCurrentPlayer(), qui devrait être plus explicite.
• Ramenant tous les accesseurs et mutateurs au niveau des fonctions appelantes, ce qui signifie que j’ai remplacé chacun de ces appels de fonction par le code qui est à l’intérieur de ces fonctions.

Ces dernières étapes sont vraiment une question de style et de préférence, donc je ne vais pas m’éterniser sur celles-ci³. Le point intéressant ici, c’est que j’ai appliqué chacune de ces étapes de manière indépendante, afin de retourner à un état stable aussi rapidement que possible, en faisant les plus petits progrès que je pouvais imaginer. C’est pourquoi cette pratique se combine très bien avec des tests automatisés, qui permettent de rapidement se rendre compte d’un bug qui se glisserait entre deux modifications!

Connaître les refactorings spécifiques aide beaucoup à définir les étapes à exécuter (mais je ne vais pas vous mentir, j’ai eu le livre sur mon bureau durant tout l’exercice). Et les plus simples d’entre eux peuvent être automatiquement exécutés par la plupart des IDE récents en appuyant sur une simple commande!

Mais cela ne signifie pas que ces recettes soient la solution ultime pour résoudre tous les anti-modèles de conception logicielle. Ainsi, chaque solution proposée doit être adaptée à votre application, son contexte, et ses fonctionnalités. En fait, le livre propose plusieurs façons d’appliquer chaque refactoring, et insiste sur le fait qu’il ne s’agit pas d’une liste exhaustive.

Atout pour le futur

Maintenant que j’ai une base de code plus propre, la mise en œuvre de la fonctionnalité mentionnée devrait être plus facile. Et même si j’ai écrit finalement plus de code, j’ai amélioré ma compréhension de ce code. Alors autant profiter de cette compréhension avant qu’elle ne s’estompe! Il y a encore quelques refactorings très utiles et faciles à effectuer: Renommer fonction et Renommer variable.

Ces briques élémentaires de chaque langage de programmation ne doivent pas être prises à la légère! Parce que nous, programmeurs, passons plus de temps à lire du code qu’à en écrire, être en mesure de comprendre rapidement ce que fait une fonction / méthode, ou quelles données contient une variable, est un avantage très précieux dans nos tâches quotidiennes. Alors montrons nous attentionné(e)s envers nos collègues (ainsi que nos futurs nous!), et trouvons des noms explicites pour chacune de ces fonctions et variables. Si je m’aperçois que j’ai écrit un commentaire au-dessus d’une telle fonction ou variable, il est probable que le nom adéquat se trouve déjà dans ce commentaire (ce qui devrait rendre le commentaire obsolète pour le coup)…

Avec tout ce temps passé à réécrire le code existant, j’ai possiblement dépassé le temps estimé pour implémenter la fonctionnalité. Je ne m’en inquiète pas trop cependant, parce que j’ai tout de même amené de la valeur dans la base de code:

• J’ai évité des bugs avant que quelqu’un d’autre (testeur ou client) ne soit tombé dessus.
• Avec une base de code plus aisément compréhensible, les autres bugs éventuels seront toujours plus faciles à repérer et à corriger
• Le refactoring est une compétence, autant que la production de fonctionnalités, et chaque fois que je le pratique, j’obtiens de meilleurs résultats plus rapidement!
• La base de code plus propre permettra sûrement d’accélérer le développement des prochaines fonctionnalités.

Si vous hésitez à faire du refactoring dans votre environnement de production, c’est tout à fait compréhensible. Je vous conseille de vous faire la main, notamment sur quelques code katas. Que sont ces katas? Cela pourrait bien être le sujet d’un prochain article…

D’ici là, continuez à coder!