Les revues de code

A quoi ça sert ?

Les revues de codes sont censées améliorer la qualité de votre application et détecter les anomalies. Cependant, elles ne sont pas appliquées systématiquement ou ont tendance à être oubliées lorsqu’il y a un peu de pression sur les délais.

Pour se donner bonne conscience on fait souvent une revue de code à la fin du projet.
C’est à ce moment-là qu’on se rend compte que certaines parties de l’application ont été mal conçues et que les conventions n’ont pas été respectées partout.
Il est à ce stade trop tard pour tout refaire, l’application devra vivre telle qu’elle car le client ne souhaite pas repayer ce qu’il a déjà payé.

Ce qu’on peut entendre

On n’a pas le temps là, on est déjà très en retard

Personne ne peut me relire car je suis le seul à connaitre cette fonctionnalité

Il n’y en a pas besoin, l’équipe est expérimentée

Ce que vous avez à y gagner

Les revues de codes ne sont pas là pour faire perdre du temps à l’équipe.
Elles vont vous permettre de

  • Détecter assez tôt une partie des anomalies,
  • vous assurer au fil de l’eau de la qualité de votre application,
  • partager les connaissances de votre équipe,
  • faire monter en compétence vos nouveaux.

Comment mettre en pratique les revues de code ?

Il n’y a pas de formule magique, c’est à chaque équipe d’adapter selon ses besoins. Voici néanmoins quelques pratiques que je vous recommande :

  • Une revue est obligatoire avant chaque commit/push, forcez-vous à jouer le jeu, même le techlead doit se faire relire. Si vous avez du mal avec ce principe, rajoutez une colonne « relecture » sur votre scrumboard.
  • Ce n’est pas toujours la même personne qui vous relit. Lorsque vous avez terminé votre développement, demandez dans le bureau une personne pour vous relire, évitez systématiquement la même personne.
  • Une relecture c’est rapide, 10 – 15 minutes au maximum sinon ça veut dire que c’est trop compliqué ou que vous avez écrit trop de code d’un coup.
  • Votre relecteur arrive, expliquez lui ce que vous avez fait et laissez-lui le clavier / souris pour qu’il inspecte le code.

Que faut-il vérifier ?

Vous pouvez partir d’une checklist, mais gardez en tête qu’il ne faut pas l’appliquer bêtement, vous devez vous imprégner du code et le comprendre. Voici quelques exemples de choses à vérifier :

  • La javadoc est présente, complète, pertinente,
  • le code est au bon endroit (dans le bon service, dans le bon package),
  • les classes, fonctions et variables ont des noms appropriés,
  • la complexité cyclomatique est faible, en clair vos fonctions ne font pas 300 lignes et ne contiennent pas des imbrications de if,
  • les tests unitaires existent, sont pertinents et passent,
  • il n’y a pas de code mort ou commenté

Si vous avez des choses à corriger suite à la relecture, vous devez rappeler votre relecteur suite à vos correctifs. C’est lui qui vous autorise à faire un commit votre travail.

6 thoughts on “Les revues de code”

  1. Une revue de code c’est utile sauf si vous mettez en place quelques pratiques de développement logiciel.

    Vous oubliez de préciser qu’une revue de code sans tests unitaires ne sert à rien.
    Une revue de code entraine un refactoring systématique du code.
    Sans tests unitaires, vous ne pouvez pas faire de refactoring efficace.

    Les commentaires dans votre code est un symptôme que votre code n’est pas assez explicite. La javadoc ne sert à rien. C’est contradictoire avec rendre son code explicite (3ème point).

    Si vous faites un peu d’extrem programming, les revues de code sont inutiles. Le pair programming remplace cet objectif.

    Faire travailler toute une équipe en rotation sur tout le produit, utiliser le développement piloté par les tests, et autoriser les développeurs à faire du pair programming, et les revues de codes deviennent inutiles.

    1. Bonjour,
      tout d’abord à propos de la Javadoc, vous dites « La javadoc ne sert à rien ».
      Si vous êtes tout seul sur votre (petit) projet et que vous êtes plutôt à l’aise avec, pourquoi pas.

      Cela dit je pense qu’il est indispensable de rédiger la javadoc sur vos fonctions métiers : d’expliquer le retour, les paramètres et de résumer le traitement effectué. Cela ne vous interdit pas de donner des noms explicites à vos fonctions. Votre code peut être explicite mais complexe (quelques fonctions chainées, des calculs, des règles métiers bien spécificiques), je n’ai pas envie d’aller lire le code source de vos fonctions pour savoir ce qu’elles font en détail, surtout que je n’ai pas forcément les sources à disposition (import de jar). La javadoc n’est pas un aveu d’échec.

      Une revue de code entraine un refactoring systématique du code.

      Ne confondez pas la revue de projet finale et la relecture de code. La revue expliquée ici sert à vérifier que les pratiques définies ont bien été appliquées, ces pratiques ne sont pas toujours respectées pour diverses raisons. Si tout est ok, la relecture va très vite, pas de refactoring (c’est le cas 75% du temps d’après mon expérience).
      La relecture se fait en binôme avec celui qui a écrit le code et un relecteur au hasard dans l’équipe.

      Nous sommes d’accord, on écrit des tests. Il est fait mention de vérifier que les tests passent, cela sous entendait qu’il y avait bien des tests.

      Le pair programming est effectivement l’étape supérieure, où la relecture de code est faite en continue. Mais pour diverses raisons il n’est pas toujours possible de travailler de cette façon, dans ce cas faire relire son code avant chaque commit est une alternative efficace.

    2. Si vous faites un peu d’extrem programming, les revues de code sont inutiles.

      Je m’inscrit en faux contre cette assertion : si j’ai une équipe de développeurs qui font du pair programming et relisent leur code c’est bien, mais si je me mets dans la peau du lead developper ou intégrateur ou architecte ou auditeur sécurité je vais quand même avoir besoin de faire des revues de code pour identifier des défaillances ou axes d’améliorations.

      Et ceci même si il n’y a pas de tests unitaires ni refactoring, ça n’empêche pas de faire des recommandations à mettre en oeuvre pour la prochaine version (donc en passant par la qualif, la recette…). Et une de ces recommandations peut être la mise en place de tests unitaires 🙂

    3. « Les commentaires dans votre code est un symptôme que votre code n’est pas assez explicite. » Une affirmation bien radicale, même Martin Fowler est moins radicale

      Almost immediately I feel the need to rebut a common misunderstanding. Such a principle is not saying that code is the only documentation. Although I’ve often heard this said of Extreme Programming – I’ve never heard the leaders of the Extreme Programming movement say this. Usually there is a need for further documentation to act as a supplement to the code.

      . Nous avons tous reprit du code qui implémente des règles métiers un peu lourdes sans commentaire … Même en nommant correctement les variables, attributs, méthodes et leurs paramètres, il me parait plus simple d’expliquer le « pourquoi » dans un commentaire que de tout miser sur du code « explicite ».

      « La javadoc ne sert à rien. » : Alors la j’aimerai que tu développes un peu ton point de vue parce que le JDK sans JavaDoc moi ca me parait improbable … En tant que consommateur d’une interface, j’aime avoir le « quoi » dans la javadoc sans avoir a parcourir tout le code …

  2. Article très intéressant, néanmoins pour ma part j’effectue une différence entre la relecture post développement de la revue de code qui va prendre un ensemble de fonctionnalités.

    Les développements s’effectuent de façon isolées et unitaires, une personne chargée de la partie couches basses (Services -> Bdd) et dans le même temps une autre chargée de la partie présentation.

    L’élément clé dans la relecture post développement est finalement de parcourir le code et de vérifier que la fonctionnalité développée ne souffre d’aucun bug fonctionnel ou oubli.
    Pour la relecture technique elle est nécessaire mais une batterie d’outil existe en partie pour contrôler les choses comme le manque de javadoc, le nommage incorrect ou trop court, la couverture des tests unitaires, …

    Pour moi la relecture à chaud doit vraiment s’orienter vers le contrôle de la fonctionnalité, si on part de l’exemple d’un formulaire de modification de profil.
    Notre service d’enregistrement prend-il en compte la saisie d’une civilité ou la suppression d’une saisie de civilité?

    si civilite != null : Enregistrement de la civilité
    sinon : Suppression de la civilité existante, si elle existe

    Bien souvent les développeurs ne prennent que le cas passant, créant de ce fait un bug fonctionnel qui remontera que bien plus tard en recette voir en production.

    Certains autres points techniques peuvent être amenés, tels que des questions d’optimisations de code, de factorisation, …

    Un autre pan, est la revue globale effectuée de façon périodique par un référent technique/expert/architecte, et qui viendra lever des problèmes purement technique et très éloignés du fonctionnel sur des sujets qui ne sont pas maîtrisés par les développeurs comme des problèmes de réentrance, fuites mémoires, erreurs communes sur des classes proxifiées (Hibernate, Spring, …), …

    La finalité est la connaissance de ces problématiques par les équipes de développement afin de ne pas les reproduire, et les amener au fil de l’eau dans les relectures de code entre développeurs.

  3. Vincent : « La javadoc ne sert à rien. C’est contradictoire avec rendre son code explicite. ».

    La javadoc sert à quelque chose puisse qu’elle a été créée pour répondre à un besoin. A titre d’exemple, le code source du langage Java est très documenté, non seulement dans les fichiers de code source eux-même, mais aussi dans des ouvrages souvent écrit par les développeurs de ce code source.
    Autres exemples : le mathématicien Donald Knuth a écrit le langage Tex, qui est très très documenté. Plus récemment, le langage scientifique Julia, plus puissant que Python sur de nombreux points, est aussi relativement bien et abondamment documenté.

    Enfin, mettre en relief une contraction entre la javadoc et un code explicite n’est un argument que pour toi. Le sens fourni par une javadoc et la signification du nom d’un objet ou d’une fonction sont distinctent, l’une complète l’autre et il n’y a pas de contractiction.

    Enfin, j’irais même un peu plus loin pour justifier l’existence d’une documentation; d’un part, en rappelant que les vocabulaires, les vocables, les écrits sur l’informatique sont souvent utiliser à tort et à travers, dans une sorte jargon dégueulasse sans véritable signification, ni valeur. D’autre part, de nombreux projets voient leur budget initial multiplier ou échouer pour des raisons de sémantique.

Laisser un commentaire

Votre adresse de messagerie ne sera pas publiée. Les champs obligatoires sont indiqués avec *

trois × 1 =