Les goûts et les odeurs
Si on demande à une assemblée de développeurs à partir de quand ils considèrent qu’une méthode est longue, les réponses vont varier : 10 lignes, 20 lignes, …
Alors même que nous sommes à peu près tous d’accord sur le fait qu’une méthode trop longue constitue du mauvais code, nous notons que nous ne sommes pas exactement d’accord sur la définition même de ce problème.
Mais y a-t-il des standards pour discerner un mauvais code d’un bon code ? Venons-en donc au fait : les bad smells.
Alors, c’est quoi un bad smell ?
Le terme vient de deux développeurs que sont Martin Fowler et Kent Beck.
Nous retrouvons l’histoire de cette naissance dans le livre Refactoring: Improving the design of existing code de Martin Fowler. Dans l’écriture de ce texte, l’auteur s’interroge sur les indications à donner pour détecter quand une action de refactoring est nécessaire. Pour lui, il est important de s’appuyer sur autre chose que sur des conceptions esthétiques.
Kent Beck lui explique alors le concept de bad smell. Il y a des codes qui sentent plus mauvais que d’autres et qui laissés ainsi mèneront à des catastrophes. Sur quoi s’appuyer pour définir un bad smell ? L’expérience.
A partir de là, Martin Fowler définit une liste de bad smells.
L’idée qui est importante, c’est qu’on cherche à faire de la qualité de code non pas pour des raisons esthétiques, mais dans le but d’éviter les bugs, de satisfaire les utilisateurs et de construire une application maintenable et robuste.
Prenons un exemple de bad smell afin d’en avoir une idée plus claire. Un que j’aime bien parce que facile à comprendre est Alternative classes with different interfaces. Il s’agit là de deux classes qui font la même chose, mais de manière différente.
Exemple d’”Alternative classes with different interfaces”
class Voiture { function roule() { //Fait avancer le véhicule } } class Auto { function avance() { //Fait avancer le véhicule } }
Cela arrive parfois quand un développeur ne s’est pas rendu compte que ce dont il avait besoin existait déjà.
Pour corriger cela, on peut donc utiliser plusieurs techniques dans le but de fusionner les classes : move method, rename method, …
Donc à chaque bad smell correspond un traitement et une liste de refactorings. Ainsi notons que le bad smell, concept créé pour le refactoring, est peut-être plus facile à repérer une fois fait que lorsqu’on le fait.
Coder proprement, c’est évident, diront certains.
Et en effet, il y a des bad smells qui sentent assez mauvais dès leur création :
- le code mort
- le code dupliqué
Cependant, il y en a aussi des moins évidents.
Et c’est là que les choses peuvent se compliquer. Si la plupart s’accorde sur le fait qu’une longue méthode ou une longue classe constitue un bad smell, on est en droit de se demander ce qu’est une longue méthode ou une longue classe.
Volontairement, Martin Fowler et Kent Beck ne donnent pas de métrique. Plusieurs outils, auteurs et speakers ont pourtant défini des métriques. Qui a raison ?
C’est là qu’intervient le ressenti et le nombre va varier en fonction des individus mais aussi plus concrètement en fonction des contextes et besoins qui parfois nous donnent l’impression qu’on peut généraliser un cas spécifique.
Et puis dans les choses moins évidentes, il y a des bad smells qui semblent parfois se contredire ou qui du moins nous montrent qu’il y a plus de gris dans la qualité d’un code que de noir ou blanc. C’est le cas par exemple des bad smells large class et lazy class. Le premier explique qu’une trop grande classe constitue un problème. L’une des solutions est d’extraire une classe ou sous-classe. Mais celle-ci ne doit pas constituer une lazy class, c’est-à-dire une classe qui ne ferait pas grand chose.
Les bad smells sont subtiles et se complètent.
Prenons un autre exemple que j’aime bien et qui illustre ce fait. Nous avons un bad smell nommé Primitive obsession. Il s’agit là d’un cas où on préfère utiliser des primitives plutôt que de petits objets, typiquement quand on préfère rassembler des données dans un tableau plutôt qu’un objet. C’est plus difficile à maintenir, à lire et c’est dommage de ne pas se servir de tous les bienfaits de la POO (Programmation Orientée Objet).
Exemple de’”Primitive obsession”
$personne = ['nom' => 'Linda', 'age' => 19]; $nomPersonne = $personne['nom'];
En parallèle, il existe un autre bad smell nommé data class. Celui-ci explique qu’une classe qui ne contient que des getters/setters prend beaucoup de place pour pas grand chose et ne devrait pas être.
Exemple de’”Data class”
class Personne { protected $nom; protected $age; public function __construct(string $nom, int $age) { $this->nom = $nom; $this->age = $age; } public function getNom(): string { return $this->nom; } public function getAge(): int { return $this->age; } }
Le problème c’est qu’en extrayant un objet d’un tableau pour supprimer un primitive obsession, on risque de créer une data class.
Alors comment faire ?
Il ne s’agit pas de remplacer un bad smell par un autre.
Le data class peut constituer une première étape dans la correction d’un primitive obsession. Mais, une classe qui contient du comportement et pas uniquement des données apporte plus de sens à notre application et doit être l’aboutissement de la correction.
Exemple de classe avec du comportement
class Personne { protected $nom; protected $age; public function __construct(string $nom, int $age) { $this->nom = $nom; $this->age = $age; } public function getNom(): string { return $this->nom; } public function getAge(): int { return $this->age; } public function speak(): void { echo “Salut”; } }
Notons d’ailleurs que les code smells et la dette technique font partie intégrante du TDD version Kent Beck et de sa manière d’aborder la conception incrémentale.
En effet, pour que le test passe au vert la première fois, on a tous les droits. Le seul critère est la rapidité du feedback.
L’étape de refactoring qui suit a ensuite vocation à supprimer la dette technique et faire émerger la “juste conception”.
C’est donc un mal nécessaire temporairement qui nous permet de trouver le bon équilibre entre pragmatisme et le design qui nous sera le plus maintenable et rentable à moyen et long terme.
La maîtrise de cette dette temporaire est capitale.
Au-delà de la subtilité des bad smells, il y a aussi le fait que certains ne font pas l’unanimité et peuvent créer des zones de conflits.
Prenons par exemple le cas du commentaire. Pour nos deux auteurs, ceux-ci quand ils se retrouvent dans le corps de fonctions représentent souvent du déodorant et avant d’en insérer un, ils préfèrent tenter de faire parler le code de lui-même en le restructurant. L’une des solutions est de faire des extract method et de donner des noms explicites aux sous-méthodes à la place des commentaires.
Exemple avec commentaire
function roule() { //Met le carburant DuCode //Met la clef DuCode //Demarre DuCode //Appuie sur la pedale DuCode }
Exemple sans commentaire
function roule() { metLeCarburant(); metLaClef(); demarre(); appuieSurLaPedale(); }
De cette manière-là, le code parle de lui-même et on n’est point forcé de lire les méthodes extraites lors d’une phase de debug si elles ne nous intéressent pas. Chaque méthode suit aussi le principe de responsabilité unique (SRP).
Cependant d’autres personnes diront qu’il n’est pas nécessaire de créer une fonction si elle n’est pas réutilisée et que ce n’est pas bon pour les performances.
Mais d’autres répondront que c’est en découpant qu’on se rend compte que telle ou telle fonction est réutilisable et qu’il est plus évident de repérer les problèmes de performance en découpant ses fonctions avec un code plus lisible.
De plus, dans le cas de beaucoup de langages comme Java par exemple, le compilateur, l’nterpréteur ou la machine virtuelle va faire des optimisations qui rendront les appels de méthodes transparents d’un point de vue performance.
Personnellement, je n’aime pas beaucoup les commentaires et je préfère découper mes fonctions en tentant de leur donner un nom explicite.
Cependant, d’une manière générale, la qualité d’un code, c’est aussi quelque chose qui se décide en équipe parce que nous écrivons pour son futur soi et son équipe. Alors, il s’agit de trouver un accord et de composer avec les attentes de ses collègues développeurs.
Conclusion
L’idée de mettre des noms sur des concepts est importante parce que comme l’expliquait Hegel, une chose n’existe que lorsqu’elle a un nom.
Cependant, c’est souvent subtile le bad smell et parfois sujet à conflit.
Et ce n’est pas toujours facile à discerner parce qu’il faut de l’expérience et que nous n’avons pas tous la même que ce soit en terme de temps ou de diversité. C’est pourquoi il s’agit de la partager comme l’ont fait initialement Martin Fowler et Kent Beck.
Il est aussi intéressant d’étendre ses connaissances vers d’autres concepts comme notamment le concept SOLID.
Liens pour en savoir plus :
Est-ce que les warnings de compilation et erreurs d’analyse statique sont également des “bad smells” pour vous ?
Bonjour Thibault. Pour les erreurs d’analyse statique, je dirais qu’elles peuvent servir à mettre le doigt sur des bad smells : duplication de code, complexité du code, … Cependant, l’analyse statique sert aussi à voir si un code est compatible à un standard et elle ne peut pas non plus tout voir.
Quant aux warnings de compilation, j’ai trop peu d’expériences en langages compilés pour émettre un avis.