Overblog Tous les blogs
Editer l'article Suivre ce blog Administration + Créer mon blog
MENU
Laurent COCAULT
Publicité

Refactoring (1) - Smelly Code

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:

Publicité
Refactoring (1) - Smelly Code

Smelly code, smelly code, how are they coding you?
Smelly code, smelly code, it's not your fault

Vous entendez "cat" ?

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.

Publicité

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.

Ken Thompson

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:

Refactoring (1) - Smelly Code
Publicité

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.

Albert Camus

Refactoring (1) - Smelly Code

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.

Refactoring (1) - Smelly 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:

  • Pour deux d'entre-elles ("x" et "a"), la concision a été poussée à l'extrême jusqu'à les rendre inexpressives,
  • Une troisième ("row") présente un nom inadapté par rapport à son contenu,
  • La quatrième ("Colonne") est non seulement incorrecte du point de vue sémantique mais apporte de plus de l'hétérogénéité dans la langue et la casse utilisées.

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.

Refactoring (1) - Smelly Code

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.

Refactoring (1) - Smelly Code

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.

Refactoring (1) - Smelly Code

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).

Refactoring (1) - Smelly Code

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".

Refactoring (1) - Smelly Code

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.

Refactoring (1) - Smelly Code

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:

  • Create a new child class to express the split-off responsibilities.
  • If the responsibilities of the original parent class no longer match its name, rename the parent.
  • Create an instance of the child class when constructing the parent and add a link from parent to child.
  • Use Move Field on each field you wish to move. Test after each move.
  • Use Move Function to move methods to the new child. Start with lower-level methods (those being called rather than calling). Test after each move.

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.

Refactoring (1) - Smelly Code

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).

Refactoring (1) - Smelly Code

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.

Refactoring (1) - Smelly Code

A suivre...

Partager cet article
Repost0
Pour être informé des derniers articles, inscrivez vous :
Commenter cet article