fabernovel loader

Jun 2, 2017 | 7 min de lecture

Tech

La revue de code façon Applidium

Benjamin Lavialle

Developer Senior


FABERNOVEL TECHNOLOGIES
La revue de code (code review) est la relecture du code d’un développeur, par un autre. De la même façon qu’un auteur, à la recherche de fautes de frappe, de grammaire, etc. demande à faire relire son oeuvre, les développeurs, soucieux de la qualité de leur code, devraient laisser leurs pairs vérifier leur travail.

Dans l’optique de produire du code de meilleur qualité chez Applidium, nous avons décidé d’intégrer la revue de code à nos projets. Et depuis, nous pensons que cela nous a permis d’améliorer la qualité de notre code, et de faire mieux communiquer les développeurs entre eux, ce qui favorise le partage de connaissances.

Pourquoi utiliser la revue de code ?

Lors de la production d’un logiciel, les bugs font inévitablement surface. Cela peut être pendant le développement, les tests qualité, ou pire, après le déploiement, provoquant une expérience utilisateur médiocre. Dans son étude sur la revue de code (en anglais), SmartBear explique qu’une revue, même rapide, permet de trouver la plupart des bugs pendant le développement. Ceci réduit ainsi le nombre d’erreurs à traiter par la Q/A ou visibles par le client, qui coûtent beaucoup d’argent.

Rapidement, on voit que la revue de code permet de :

  • augmenter la qualité du code
  • réduire le nombre d’anomalies
  • améliorer la communication entre développeurs sur le contenu du projet
  • entraîner les développeurs juniors

Il en découle indirectement :

  • une réduction de la durée des cycles de développement/test
  • une réduction de l’impact sur le support technique
  • une augmentation de la satisfaction client
  • la production de code plus maintenable
  • un renforcement des connaissances du relecteur, qui est forcé de formaliser des critères de qualité

Comment est-ce que nous revoyons le code chez Applidium ?

Nous utilisons un outil appelé Gerrit qui propose une interface web pour interagir (noter, commenter, répondre) avec les changements. La revue de code est faite à l’aide de notes allant de -2 à 2 :

  • -2 Ne devrait pas être intégré
  • -1 Je préfèrerais qu’il ne soit pas intégré, en l’état
  • 0 Pas de note
  • 1 Me paraît bien, mais quelqu’un d’autre doit valider
  • 2 Me paraît bien, approuvé

Lors de sa revue, le développeur doit analyser en détail les changements. Voici quelques conseils pour faire une analyse de qualité :

Règles d’écriture (Coding Style)

Le premier critère d’acceptation est le respect des règles d’écriture de code de l’entreprise. En effet, le coding style est un ensemble de règles provenant des standards de l’industrie ; elles sont définies pour structurer le code, le rendre homogène et donc plus compréhensible. Le fait de ne pas valider un changement à cause d’un détail de syntaxe peut paraître strict ; mais il est important que l’intégralité du code respecte la même syntaxe, cela augmente la lisibilité et ainsi la maintenabilité du code. Cette étape de validation peut être automatisée par l’intégration continue. Chez Applidium, nous utilisons SwiftLint pour Swift sur iOS et Lint pour Android, sur Sonar pour automatiser cette étape.

Est-ce que le code est propre et sûr ?

C’est une observation générale sur la qualité du code ; globalement, est-ce que le code respecte les critères SOLID ? Le relecteur devrait aussi vérifier les éventuelles duplications de code, gestions d’erreur manquantes, etc.

Il est aussi important de vérifier la sécurité des changements sur l’API : par exemple, une nouvelle méthode peut être utilisée correctement dans le contexte actuel, mais permettre à autrui une mauvaise utilisation plus tard, qui mettrait en péril le fonctionnement de cette partie du code.

De plus, tout changement doit être considéré comme une opportunité de nettoyer le code vieillissant ; il faut toujours se dire que le code doit être laissé dans un meilleur état que celui dans lequel il était.

Est-ce que le code est fonctionnel ?

Observer la syntaxe, et la qualité générale du code n’est pas suffisant ; le relecteur devrait aussi contrôler les fonctionnalités, s’assurer qu’il n’y ai pas de potentiel bug ou risque. Apprécier l’élégance et juger l’efficacité sont de bon moyens de motiver l’apprentissage des développeurs tout en produisant du code de meilleure qualité.

Le code est-il compréhensible ?

Le relecteur doit être capable de comprendre tout seul le changement (et plus généralement le code) ; si il/elle n’en est pas capable sans explication, les suivants risquent aussi d’avoir des soucis de compréhension sur cette partie. Parmi les critères de compréhension du code on peut prendre en compte les choix judicieux de nom de méthodes et variables, le fait de garder des fonctions et classes de taille correcte.

Il est important de rappeler que le premier relecteur reste celui qui écrit le code : avant de soumettre le changement à la relecture, il est primordial de le vérifier (est-ce qu’il ne reste pas des variables de débogage ? Est-ce que les commentaires sont propres ? Ainsi que les critères précédents).

Jouer avec la fonctionnalité

Lors de la relecture d’une fonctionnalité, étudier chaque changement séparément est nécessaire, mais considérer la fonctionnalité dans son ensemble est aussi important. En effet, c’est une bonne habitude de parcourir le code, suivre son exécution. Essayer de comparer l’architecture actuelle à celle que l’on aurait choisi si on avait dû développer soi-même aide à comprendre son fonctionnement et mesurer son efficacité.

Prendre son temps

Il est inefficace de prendre la revue comme une obligation effectuée mécaniquement car c’est une étape importante du développement. Il n’y a aucun intérêt à bâcler cette tâche ; si le développeur n’est pas prêt à prendre assez de temps pour l’effectuer avec minutie, autant remettre à plus tard.

Faut-il relire toutes les modifications ?

Dans certains cas, les changements peuvent paraître évidents et redondants (par exemple le fait de renommer une variable dans tout un projet, mettre à jour une ressource, etc.). C’est au relecteur de décider si un parcours intégral vaut la peine. Pour faire son choix, il est important de se rappeler que les changements, même évidents, ne sont pas à l’abri d’une erreur d’inattention, ainsi que le point précédent : si la relecture n’est pas faite avec attention, c’est une perte de temps.

Une bonne ambiance est nécessaire

Il est important de rester ouvert d’esprit, que ce soit en relisant ou en étant relu.

Pour cela, les commentaires devraient toujours être expliqués : pourquoi est ce que le changement n’est pas acceptable ? Et comment l’améliorer.

De façon générale, les gens n’apprécient pas qu’on leur dise quoi faire ; le fait d’expliquer et de soulever des questions offre plus de chances d’intéresser et permet de mieux faire accepter ses remarques. De l’autre côté, il ne faut pas prendre la relecture personnellement ; ce n’est pas la personne qui est jugée, mais seulement son travail qui est relu. Lorsque l’on reçoit un commentaire, le fait de rester ouvert d’esprit et de communiquer permet de lancer des discussions intéressantes pendant lesquelles chacun progresse.

Et pour finir, il est, bien sûr, possible de laisser des commentaires positifs lors de la relecture.

Attention à prendre soin du relecteur

Comme le relecteur reprendra probablement la pile de changements telle une histoire, les uns après les autres ; il ou elle n’appréciera pas les changements de direction d’implémentation, et risque de perdre le fil du raisonnement. C’est pourquoi il est important de garder un historique propre avant de soumettre ses changements à la relecture.

Réduire les discussions autour de la “zone grise”

Les zones grises sont des sujets subjectifs sur lesquels des développeurs ont des avis divergents. La discussion diverge généralement vers un débat sans fin, inconstructif dans lequel chacun campe sur ses positions. C’est pourquoi il est important d’expliquer clairement ses commentaires de relecture, cette recherche de clarté aide souvent à trouver de la légitimité sur le sujet. En cas d’échec dans la recherche d’explication clair sur la non-validité d’un changement, celui-ci devrait être considéré comme acceptable.

Une fois que les développeurs concernés se sont mis d’accord sur un de ces sujets, il est important d’en informer le reste de l’équipe, d’expliquer le cas, et de mettre à jour les règles pour ne pas retomber dessus.

Il est autorisé de ne pas être sûr

En cas d’incertitude sur un changement, il est toujours possible de mettre une note +1 (le changement me paraît bien, mais quelqu’un d’autre doit valider). C’est une meilleure idée que de valider un changement qui n’est pas entièrement compris, ou dont le rayon d’action n’est pas totalement clair.

La relecture est un travail d’équipe

Relire et gérer les changements ainsi que leurs branches devient rapidement un casse-tête lorsque le nombre de développeurs sur le même projet augmente ; c’est pourquoi il est important que chacun reste moteur dans la gestion de ses propres changements et volontaire dans la relecture de ceux des autres.

Conclusion

La revue de code peut sembler chronophage, peu utile, voire ennuyeuse. En pratique elle l’est ; à moins qu’elle ne soit faite dans le but d’apprendre de nouvelles techniques, en prêtant attention aux détails et en étant motivé et positif. Il est nécessaire de se plonger dans la relecture, prendre en compte tout ce que chaque changement implique, communiquer sur les problèmes rencontrés et les bonnes habitudes que relecteur et relu peuvent rencontrer ; cela crée un bon vecteur de partage d’information au sein de l’équipe de développement. Chez Applidium, nous considérons la relecture de code comme un moyen et une opportunité d’améliorer la qualité du code produit ainsi que le niveau technique des développeurs ; c’est donc pour nous une étape primordiale du développement.

Cet article vous a plu et vous souhaitez en parler avec nous ?

Contactez-nous !
logo business unit

FABERNOVEL TECHNOLOGIES

150 talents pour répondre aux défis technologiques de la transformation digitale.

à lire