28 Décembre 2019
Clean Code est un ouvrage publié par Robert C. Martin en août 2008, devenu en une décennie un ouvrage de référence et le point de départ d'un mouvement visant à promouvoir les valeurs de l'artisanat logiciel. C'est aussi la source d'inspiration principale des sessions de partage de bonnes pratiques pour le développement logiciel qui sont organisées au sein de la communauté des ingénieurs logiciel de Capgemini. L'exemple qui sert à illustrer ce qu'on peut sans hésitation qualifier de mauvais code est le suivant:
Smelly code, smelly code, how are they coding you?
Smelly code, smelly code, it's not your fault
Cet exemple concentre suffisamment de mauvaises pratiques pour me sembler être le meilleur point départ permettant de mettre en pratique les bons conseils d'une autre référence mondiale du développement logiciel: les techniques de "Refactoring" de Martin Fowler. Depuis sa première édition en 2002, cet ouvrage propose un ensemble de techniques permettant d'identifier des "erreurs" de développement et suggère les techniques de correction associées. Le terme "erreur" mérite ici d'être précisé puisqu'il ne s'agit pas à proprement parler d'anomalies logicielles entraînant un comportement erroné du programme correspondant, mais plutôt de défauts de structure qui mettent en péril la maintenabilité du code et contreviennent en général aux bonnes pratiques que l'oncle Bob devait synthétiser par la suite.
Commençons par les mauvaises pratiques que Fowler appelle "code smells". Le terme est passé dans le langage courant des informaticiens; pourtant, peu nombreux sont ceux qui connaissent effectivement la liste des "smells". Une liste peut être trouvée sur la page Wikipédia dédiée au concept.
Avant d'appliquer les recettes de Martin Fowler, un petit nettoyage préliminaire s'impose. Un avertissement de notre IDE (ici j'utilise Eclipse) met en évidence le fait que la ligne 13 initialise une variable qui n'est d'aucune utilise. Notre premier "refacto", s'il ne correspond pas à proprement parler à une technique proposée par Fowler, consiste donc à supprimer cette ligne inutile.
One of my most productive days was throwing away 1000 lines of code.
Mais le véritable nettoyage initial ne se limite pas à la suppression d'une instruction. Le problème initial qu'il faut ici adresser est l'absence de tests unitaires. En effet, pour mener des activités de "refactoring", il faut veiller à se protéger des régressions auxquelles nos interventions nous exposent. Or, l'utilisation d'une fonction "main" produisant du contenu sur la sortie standard permet difficilement de mettre en place un test respectant les principes de "self-testing code", c'est à dire un test vérifiant automatiquement les sorties produites. Cette activité d'initialisation des tests doit donc nécessairement passer par une première activité de "refactoring" que Fowler qualifie de "Extract Function". En utilisation l'IDE Eclipse, cette technique peut être appliquée via le raccourci Alt+Shift+M qui propose l'écran suivant:
Il est intéressant de souligner ici l'importance pour un développeur de connaître son IDE, tout comme un bon artisan connait ses outils. Pour ma part, je me suis longtemps contenté de n'utiliser dans le menu "Refactor" d'Eclipse que la fonction "Rename" (Alt+Shift+R). La lecture de l'ouvrage de Fowler m'a permis de mieux appréhender les possibilité offertes par cet IDE.
On pourra noter que la fenêtre de dialogue proposée par Eclipse nous invite à donner un nom au code extrait. On en vient ici à l'un des piliers du "Clean Code": nommer correctement et explicitement les choses.
Mal nommer un objet c’est ajouter au malheur de ce monde.
Un avantage immédiat de cette première extraction est d'avoir permis de mieux identifier la portée des variables; il ne fait aucun doute que cette isolation limitera la portée du "refacto" qui consistera à renommer des variables manifestement hasardeuses. Mais avant d'en venir là, il nous faut introduire les tests qui permettront de sécuriser la refonte de notre code.
Un seul cas de test est proposé dans cette implémentation. Si nous développions en TDD, nous serions amenés à produire plusieurs tests pour cette fonction, en commençant typiquement par les cas particuliers ou les cas aux limites. Ici, l'objectif à court terme est de prévenir toute régression par rapport à un comportement qui n'est défini que pour une valeur.
Commençons par la détection de notre premier "smell". Le plus évident me semble être "Mysterious Name". Les arguments et variables présentent plusieurs défauts de nommage:
Le "refacto" à appliquer pour corriger ces "smells" est tout simplement "Rename Variable". Là encore, notre IDE va nous faciliter le travail et il ne nous reste plus qu'à trouver un nom à la fois concis et expressif.
Toujours dans un souci d'apporter de la sémantique au code existant, on pourra appliquer "Replace Magic Literal" afin d'extraire les valeurs constantes du code en les nommant. Eclipse propose la fenêtre suivante lorsqu'on applique l'outil correspondant sur la première valeur "60" de la fonction formatTime, qui correspond au nombre de secondes dans une minute.
Il est intéressant de noter que la visibilité par défaut proposée pour les constantes est "private". On a trop souvent tendance à exposer les constantes en visibilité publique. Or il est inutile, voire dangereux, d'exposer une constante dont l'usage externe n'est pas justifié.
L'extraction de constante suivante concerne le nombre de minutes dans une heure. Il s'agit du second "60" de la fonction de calcul de la variable "minutes". Mais une fois qu'on a procédé à cette seconde extraction, il nous reste des littéraux qui correspondent à des constantes déjà extraites.
Dans ce cas, il convient de remplacer les valeurs littérales par les constantes adéquates manuellement. Une fois ce travail réalisé, on peut encore ajouter de la sémantique sur la valeur "60*60" que notre "refacto" a changé en "MINUTES_BY_HOUR * SECONDS_BY_MINUTE" et qui correspond à un nombre de secondes par heure (la fonction "Extract Constant" d'Eclipse fonctionne également dans ce cas).
Notre code a largement gagné en expressivité et on pourrait s'arrêter là. Néanmoins, un point doit encore attirer notre attention: nous avons principalement travaillé sur l'extraction des heures, minutes et secondes et non sur leur rendu. Au moment se s'attaquer à cette partie du code, nous pouvons douter de la pertinence d'adresser ces deux problématiques au sein d'une unique fonction. Il n'est en effet pas du tout exclu que la mise en forme soit amenée à changer alors même que la logique d'extraction reste stable dans le temps. Par ailleurs, on pourra noter que les différentes fonctions de calcul sont indépendantes, ce qui permet de répéter la transformation "Extract function".
L'extraction des fonctions d'analyse permet d'améliorer la séparation des préoccupations mais n'est pas complètement achevée. L'application de la transformation "Separate Query from Modifier" permet de réellement séparer les deux parties. Mais procéder à une telle séparation suppose de disposer d'un état, ce qui n'est pas le cas avec une implémentation statique. Avant d'appliquer cette transformation, un passage par des méthodes non statiques et l'introduction d'un état s'impose.
Note: les détracteurs de la programmation orientée objet viendront ici souligner cette inclinaison du paradigme à injecter des états qui ne sont pas absolument nécessaires. Dans ce cas précis, c'est probablement vrai puisqu'on aura pu répondre au besoin et grandement améliorer la qualité du code en restant fonctionnel.
La multiplication des méthodes au niveau du programme amène la question de la cohérence d'ensemble. Les quatre méthodes introduites constituent un ensemble cohérent qui se distingue de la fonction principale d'appel et qui porte une spécificité, celle de la valeur utilisée. L'application de "Extract class" permettra de rematérialiser ce périmètre consistant. Malheureusement, la fonctionnalité correspondante proposée par Eclipse ne permet pas la mise en oeuvre des étapes suivantes:
La place laissée à l'appréciation du développeur explique certainement en quoi une automatisation complète s'avère difficile. On procèdera donc manuellement en créant une classe Clock et en adaptant les tests unitaires et la fonction principale.
Une des dernières étapes de la refonte vise à s'intéresser à la fonction de mise en forme de l'heure. Le format est en effet tout à fait discutable et on aurait pu s'attendre à un format plus classique, tel que "hh:mm:ss". La modification suivante consiste donc à ajuster le test unitaire puis à remplacer la concaténation actuelle par un appel à une fonction de l'API standard Java (on s'appuiera ici sur la classe MessageFormat pour respecter l'algorithmie du code initial, bien que l'usage de SimpleDateFormat, avec une implémentation radicalement différente, aurait in fine été plus pertinent).
Pour clore ce nettoyage, intéressons-nous à la question des commentaires. Nous avons jusqu'à présent conservé le commentaire initial et nous n'avons pas introduit de nouveau commentaire alors même que nous avons largement changé la structure logicielle. Une bonne partie des éléments que nous avons fait évoluer n'ont pas besoin d'être commentés dans la mesure où leur nommage est explicite. In fine, l'ajout de commentaires inutiles reviendrait à créer une "smell" alors que le but de notre démarche est de les éliminer (cf. "no comment"). Les commentaires qu'il est ici légitime d'ajouter sont ceux qui documentent les interfaces ou explicitent nos intentions. C'est pour répondre au premier objectif que nous pouvons ajouter quelques commentaires sur le constructeur de Clock et sur sa méthode formatTime.
A suivre...