fbpx

Il y a près de 10 ans, un manager m’a dit « Tu as un degré d’exigence trop élevé. » faisant référence à mes nombreux retours lors des revues de code.
A l’époque, je faisais du Cobol, mais depuis je suis passé par Java, Javascript et PHP. Et je vous rassure, mes revues de code sont toujours aussi « exigeantes ».

Un exemple

Le cas que j’aime bien présenter pour expliquer ce concept est en Java.
L’architecte en charge du projet avait demandé à un petit service Web qui récupère diverses informations de la requête (j’ai oublié la raison exacte de cette demande, mais passons). Il s’agissait de lire un header, un cookie et peut-être une ou deux autres informations.

Voici (en substance) la première version du code proposé par un collègue (la technologie utilisée est du Spring MVC):


OK, ce code fonctionne, il fournit des valeurs par défaut, bref on pourrait s’arrêter là. Sauf que l’on n’utilise pas toutes les possibilités de Spring MVC. Voici la ré-écriture que je propose :

Fonctionnellement, les deux codes font la même chose. Mais techniquement, on a supprimé 20 lignes de code et l’on s’appuie désormais sur le savoir-faire des contributeurs de Spring pour lire les valeurs, plutôt que sur un bout de code trouvé sur StackOverflow.

Pourquoi ?

Tout cela est très bien, mais hormis de faire plaisir aux gens exigeants, à quoi ça sert de faire tout ça ? Je vous propose d’analyser ce qu’il se passe lorsque l’on ne fait pas ce genre de retours.

Il y a peu, en faisant une revue de code, je me dis qu’il y a une chose qui ne me plaît pas trop. Pour résumer, on stocke le numéro de téléphone dans un seul champ en base, mais on demande deux informations dans le formulaire : le code pays et le numéro de téléphone. Pour cela, on concatène en sortie du formulaire et au retour sur celui-ci on découpe le champ en deux.

Lorsque l’on utilise le téléphone auprès d’un service externe, on a besoin d’une seule chaîne de caractères contenant les deux informations, d’où l’idée de les stocker déjà prêtes à l’utilisation. Ça donne beaucoup de code pour un bout de formulaire, mais ne voulant pas faire perdre de temps au collègue, je laisse passer.

Bilan, le collègue reviendra 3 fois sur le code pour différents cas non prévus, avant de stocker deux champs en base et de simplifier le code autour du formulaire. Pour cela, on a dû déplacer la concaténation lors de l’appel au service externe mais on n’a plus besoin de penser au découpage.

Conclusion

Mon objectif ici n’était ni de vous apprendre Spring MVC, ni de vous raconter les boulettes Iteracodiennes. Ce qu’il faut retenir de ces anecdoctes, c’est qu’elles ont un point commun. Elles concernent du code que je n’ai pas écrit. Je paraphraserais Venkat Subramaniam en disant que l’on est souvent meilleur à repérer le mauvais code des autres que le sien.
Que l’on choisisse le pair programming ou la revue, il est important qu’un oeil extérieur regarde votre code.

Cela permet d’une part d’éviter les bugs mais aussi de partager la connaissance et l’appropriation du projet.