24 Fév 2016

Ce que la revue de code m’a apporté

Depuis plus d’un an, je pratique la revue de code collective. Je souhaitais vous partager mon sentiment sur cette pratique, et faire un tour d’horizon de tous les obstacles que j’ai pu rencontrer. Je vous détaillerai mon état d’esprit avant, pendant et après, la mise en place de la revue de code. Aujourd’hui, la revue de code est une étape obligatoire de notre Kanban, chaque user story, chaque ligne de code qui part en production est relue par l’équipe.

 

Avant la revue de code

Le code que je produisais m’appartenait, c’était mon code et j’en étais fier, j’avais pris du temps pour le faire et j’étais confiant quant à sa qualité. Je n’étais pas à l’abri de quelques bugs, mais je le maîtrisais, j’étais certain de pouvoir les résoudre rapidement.

Parfois, je demandais de l’aide pour développer une fonctionnalité, au Tech Leader ou à un autre développeur du projet, nous relisions mon code et les questions étaient souvent les mêmes :

– Mais qu’est-ce que tu as voulu faire ?
– Bah, je veux appeler le repository pour récupérer les informations du client et les afficher sur ma page
– Ok, c’est déjà un peu plus clair, mais ca veut dire quoi cette variable: “rpc”
– Bah, c’est repositoryClient

L’intention de mon code n’était pas claire pour lui ! Et moi qui le croyais simple.

Du coup, suite au passage du développeur,  je réalisais seul un peu de ménage, je renomme quelques variables, je déplace du code, je le rends un peu plus réutilisable (même si, il faut l’avouer c’était un peu frustrant, mon code n’était pas si compliqué, je le comprenais parfaitement).

Mais ce fameux nettoyage de code entraînait souvent des bugs.

– On a identifié un problème en recette, tu peux regarder ?
– Ok, pas de problème. C’est à quel endroit ?
– C’est sur le page client, on arrive plus à récupèrer les informations du client
– Ok, je te tiens au courant.

Après quelques recherches, je mettais enfin la main sur le problème, souvent mineur, une variable mal nommée et oubliée lors de cette phase de refactoring. Et tout ça pourquoi ? Parce qu’un autre développeur n’avait pas compris ce que je voulais dire.

Du coup, j’évitais de montrer du code, parce qu’en plus de recevoir des critiques, cela entraînait des bugs, et ça, je ne voulais pas créer des bugs. Je voulais faire un code de qualité.

 

Un code de qualité, sérieusement ?

Sauf que j’avais tout faux, vouloir faire un code de qualité, sans le partager, sans écouter les recommandations des autres membres de l’équipe , seul dans mon coin, c’est le meilleur moyen de faire du code médiocre, non réutilisable et souvent impossible à maintenir.

On a tous un jour, repris du code en pestant,

Mais qui a bien pu écrire ce code, je ne comprends rien, ca veut dire quoi cette variable “rpC”, pourquoi ce code est écrit en Français ? On ne devait pas l’écrire en anglais ?

Il ne fallait pas 2 minutes pour trouver un maximum de défauts. Une recherche sur l’historique du fichier, et je mettais la main sur notre coupable.

Et là, SURPRISE, le fameux développeur, c’était moi, comment avais-je pu faire un code avec autant de défauts ? A l’époque il était super, il était beau, efficace et aujourd’hui, il est illisible. Oui sauf que plusieurs mois sont passés, ma façon de coder à évolué et “rpC” ça ne veut plus rien dire pour moi. Je n’ai même pas reconnu mon code.

Mais comment faire pour avoir un code de qualité ? Maintenable et compréhensible par tous (moi y compris) ?

Pour moi, la réponse à cette question c’est la revue de code collective.

 

La revue de code

Qu’est-ce que la revue de code ? Pour faire simple, vous prenez des développeurs, du code, vous les enfermez dans une pièce, vous secouez le tout et vous comptez le nombre de “WTF” à la minute, l’unité d’usage dans cette pièce.

wtfcode

Le but est de trouver un maximum de défauts sur le code présenté, de partager et diffuser un standard. Présenté ainsi, je me suis dis c’est surtout cool tout ça, mais quel développeur serait assez fou pour accepter de remettre en cause son code, sur lequel il a parfois passé énormément de temps. Et n’oublions pas la peur d’être jugé par d’autres personnes qui peut en refroidir plus d’un.

 

Se détacher de son code

Mais alors comment faire pour qu’un développeur qui présente son code, ne claque pas la porte après 2 minutes de revue de code ?

J’ai du faire un gros travail sur moi-même. Au tout début, lorsque l’on critiquait MON code, on me critiquait moi. Avant que je comprenne. Non, ce code n’était définitivement pas le mien. Au moment où je le présente, il devient celui de l’équipe. Il n’est plus “mon” œuvre, mais “notre” œuvre. Il ne fais pas “ma” fierté, mais celle de toute mon équipe.
Les remarques des autres développeurs font parfois très mal.

“C’est de la m*****, ton code pourquoi tu as fait ça c’est pourri…”

Mais je ne faisais pas mieux, je n’hésitais pas à critiquer de manière violente le code que l’on pouvait me présenter, après tout je souffrais de leurs remarques, pourquoi pas eux ? Encore une fois, j’avais tout faux.

Pour éviter qu’elle ne se transforme en un véritable pugilat et que finalement plus personne n’ose présenter du code, notre équipe a affiché dans la salle où l’on réalise les revues de code un poster où il est écrit :duraveclecode

“Dur avec le code, doux avec les gens”

 

Ce poster peut paraitre simple, mais il a pour but de rappeler à l’assistance que le but de la revue de code est de trouver des défauts et non d’assassiner nos collègues de bureau gratuitement. Et si on assassine tout le monde, il n’y aura plus personne aux prochaines revues, donc on ne trouvera plus de défauts et en plus on se sera mis à dos les autres développeurs.

Je reformule donc mes phrases, au lieu de dire “ton code c’est de la merde”, je dis plutôt : “Mon sentiment est qu’il y a un défaut à la ligne suivante” ou “ce code ne respecte pas nos standards” et je me dis que c’est quand même plus sympa ce genre de remarque. Au fil des revues, on se détache progressivement du code, il n’y a plus d’attaques directes.

Pendant ces revues, beaucoup de défauts sont alors remontés, l’équipe a donc mis en place un standard que nous laissons évoluer au fil de nos échanges.

 

Partage et diffusion d’un standard d’équipe

La mise en place de ce standard m’a fait prendre conscience qu’avant la mise en place d’un standard unique, chaque développeur, chaque projet, chaque équipe avait son propre standard, certains écrivent tout en anglais, d’autres en Français, et d’autres même en franglais.

On s’est donc tous regroupés dans une salle et on a commencé à écrire un standard commun à toute l’équipe.

Un des premiers standard à émerger est que le code doit être écrit en TDD.

Autre exemple, la notion de client peut s’appeler : client, customer, user, on décide donc que désormais quand on parlera de client dans nos applications on utilisera le mot customer.
On évite également de raccourcir les termes.
On ne fait plus de franglais (surtout sur les nouveaux projets, les projets dit legacy, on est conscient que le travail est plus difficile).

Notre fameuse variable “rpC” deviendra donc : “customerRepository”.
Tout de suite, c’est plus parlant.

Le standard étant connu de tous, il servira de base lors de nos séances de revues de code et chaque ligne qui ne respectera le standard selon la gravité de l’erreur, sera une erreur “typo” ou “technique”.

Pendant les revues de code, certains points sont sujets à être débattus, soit le modérateur de la revue tranche, soit un échange est nécessaire et on fait évoluer notre standard.

J’ai pris alors conscience, que je ne pouvais plus coder à ma manière, sans consulter les autres développeurs.

 

Prise de conscience

Après plusieurs revues de code, difficiles, j’ai pris conscience, qu’en tant que développeur, je ne pouvais faire du code seul dans mon coin. Ce code devait respecter le standard établi par l’équipe. Je savais désormais que si je ne pratiquais pas le TDD, le clean code, je serais “snipé” en revue de code.

Rubber_duck_assisting_with_debugging

Essayez ce petit exercice, prenez 5 lignes de votre code, réunissez 5 autres développeurs, ces derniers ne doivent pas voir votre code et lisez votre code à voix haute. Les développeurs, devront alors vous décrire l’intention de votre code. Vous risquez d’être surpris, par ce que les développeurs ont compris de votre code !

Cf : La méthode du canard en plastique

 

Et maintenant ?

Dès que je rencontre un point de blocage, je sollicite de l’aide, je ne cherche plus à cacher mon code, de toute façon, les autres développeurs mettront le doigt dessus en revue de code.

Si mon code n’est pas clair, je le réécris et je suis confiant, car mon code est protégé par un harnais de tests, il est sous contrôle.

Je pratique également beaucoup plus de pair programming, ce qui me permet de me détacher encore plus du code, car ce code on le pense et on l’écrit à deux, il respecte le standard de l’équipe.
Pendant la revue, je n’oublie pas qu’il faut être dur avec le code et doux avec les gens, je formule donc correctement mes phrases, même s’il faut l’avouer il m’arrive toujours de dire parfois “c’est quoi ce code pourri”, il faut savoir quelle personne est prête à recevoir ce genre de feedback.

Je ne peux pas vous dire qu’aujourd’hui, je n’ai plus peur de présenter du code et d’être jugé par d’autres développeurs et que j’accepte toutes les critiques. C’est faux, mais je m’attelle à corriger ce défaut de revues de code en revues de code.

 

Pour aller plus loin

Les 10 commandements de l’egoless programming (EN)

OCTO : Quel format pour votre revue de code?

Strip-Code-review-650-final

Share
  • Merci pour l’article, je partage le point de vue des bénéfices immenses de la revue de code.

    Comme tu le sais, je suis très attaché à compléter les revues de code synchrones (pair review ou en équipe) par les revues de code asynchrones comme les pull-request très utilisées dans l’open source.

    J’espère que tu pourras nous faire un retour d’XP sur les revues de code asynchrones en utilisant les PR 😉

    • Julien Hatzig

      Avec un TFS 2013, il est possible de le faire en mode asynchrone, malheureusement tous les projets ne sont pas sous TFS 2013.

  • Fabien GOGUILLON

    Merci pour ce retour d’expérience très enrichissant.