Quel est le but d'une révision du code

76

Je suis en train d'essayer de vendre mon organisation sur la valeur des revues de code. J'ai travaillé à plusieurs endroits où ils travaillaient. Je les ai vus habitués à des choix de styles et à des choix fonctionnels, et je les ai vus utilisés comme un simple contrôle pour s'assurer que rien de dangereux n'était mis en œuvre. Mon sentiment est que le but le plus efficace se situe quelque part entre les deux options.

Alors, quel est le but d'une révision du code?

code-reviews theory SoylentGray
la source
16
Connexes (sur le dépassement de pile): le but des
révisions
16
- Comment sauriez-vous si vous avez écrit du code lisible et facilement maintenable? - Votre pair vous dit après avoir examiné le code. Raison: vous ne pouvez pas déterminer cela vous-même, car vous en savez plus que l'auteur dit simplement le code. Un ordinateur ne peut pas vous dire, pour les mêmes raisons qu'il ne peut pas savoir si un tableau est un art ou non. Par conséquent, vous avez besoin d'un autre humain capable de gérer le logiciel pour examiner ce que vous avez écrit et donner son opinion. Le nom officiel de ce processus est "Peer Review" .
Gnat
3
"Quel est le but d'une révision de code?" empêcher les développeurs d'écrire du code terribad et les orienter dans la bonne direction.
zzzzBov
7
Il semble que Code Review ait une réponse indirecte à cette question .. parcourez les questions et réponses, l'objectif d'une révision de code devient assez évident :)
Mathieu Guindon
3
Je me demande combien de fois les programmeurs ont découvert des bugs dans leur propre code simplement par le biais du processus simple consistant à expliquer leur code lors d'une révision?
Hache-inactif

Réponses:

75

Il existe plusieurs raisons pour lesquelles vous souhaitez procéder à une révision du code:

  • Education d'autres développeurs. Assurez-vous que tout le monde voit la modification associée à un correctif ou à une amélioration afin de pouvoir comprendre le reste du logiciel. Ceci est particulièrement utile lorsque des personnes travaillent sur des composants devant être intégrés ou sur des systèmes complexes où une personne peut rester pendant de longues périodes sans consulter certains modules.
  • Trouver des défauts ou des possibilités d'amélioration. Le code livrable ainsi que le code de test et les données peuvent être examinés pour détecter les faiblesses. Cela garantit que le code de test est robuste et valide, ainsi que la conception et la mise en œuvre sont cohérentes dans toute l'application. Si des modifications supplémentaires doivent être apportées, l'opportunité est saisie plus près du point d'entrée.

Il existe plusieurs analyses de rentabilisation pour effectuer des examens:

  • Trouver des défauts ou des problèmes qui devraient être retravaillés plus près de leur injection. C'est moins cher
  • Compréhension partagée du système et formation croisée. Moins de temps pour un développeur de se mettre au diapason pour apporter des modifications.
  • Identification des améliorations possibles du système.
  • Ouvrir la mise en œuvre pour s'assurer que les testeurs fournissent une couverture adéquate. Transformer une boîte noire en une boîte grise ou blanche du point de vue des tests.

Si vous souhaitez en savoir plus sur les avantages des stratégies d’examen par les pairs et sur les stratégies de mise en œuvre correspondantes, je vous conseillerais de consulter les évaluations par les pairs dans Logiciel: Guide pratique de Karl Wiegers .

Thomas Owens
la source
7
+1, je pense que vous avez bien repéré les points principaux, avec les bonnes priorités. Garder la conception cohérente lorsque vous travaillez avec des collègues qui trouvent constamment des solutions "WTF" très créatives ne peuvent être atteints que par des révisions de code régulières.
Doc Brown
Nous procédons à des révisions de code dans notre code JavaScript, principalement pour nous assurer que les développeurs respectent les normes énoncées, en utilisant le modèle défini lors de la conception des modules, en utilisant les composants fournis et en ne commençant pas à coder ninja (intentionnellement ou non) autour de problèmes déjà résolus. pour. Ils sont également intéressants pour repérer quelqu'un écrivant accidentellement le thiscontexte, ne l'utilisant pas .hasOwnPropertydans des endroits où il devrait être, etc., etc. - Donc, principalement pour les normes. Dans un langage géré comme C #, vous avez bien sûr moins de raisons de choisir des langages dynamiques.
Nope
1
Jusqu'ici, votre réponse fait uniquement allusion aux améliorations apportées à la correction du code. Il n'aborde pas la lisibilité / la maintenabilité du code, qui peut rarement être quantifié avec précision par le développeur en développement.
ArTs
51

Les revues de code sont un outil de transfert de connaissances .

  • Lorsque les développeurs examinent le code de chacun, ils se familiarisent avec tous les domaines du système. Cela réduit le facteur de bus d'un projet et rend les développeurs plus efficaces lorsqu'ils doivent effectuer une maintenance sur une partie du système qu'ils n'ont pas écrite.

  • Lorsqu'un programmeur junior examine le code d'un senior, il peut apprendre des astuces, sinon il n'a appris que par expérience. Cela peut également servir de correctif contre un code trop compliqué.

    Un examen approfondi du code nécessitera des contrôles fréquents sur divers documents. C'est un excellent moyen d'apprendre un langage ou une API.

  • Lorsqu'un programmeur expérimenté examine le code d'un junior, c'est l'occasion de résoudre les problèmes avant qu'ils ne se traduisent par une dette technique. Une révision de code peut constituer un bon cadre pour le mentorat des programmeurs débutants.

Les revues de code ne concernent pas:

  • … Trouver des bugs. C'est à quoi servent les tests. Il arrivera encore fréquemment qu'une revue de code trouve un problème.

  • … Corrige les problèmes de style - choisissez un style et utilisez des outils de formatage automatisés pour le faire respecter. Mais il y a beaucoup de choses qu'un outil automatisé ne peut pas vérifier. Les revues de code sont un bon endroit pour s'assurer que le code est suffisamment documenté ou auto-documenté.

Amon
la source
2
Je ne suis pas tout à fait d'accord avec votre dernier point concernant les problèmes de style, mais nous venons d'avoir une expérience pénible de la révision du code d'un développeur junior. forcé .... waaaaaay trop de déclarations if pour les cas difficiles etc; problèmes que, oui, vous pourriez faire trouver un ordinateur dans certains cas, mais la plupart ne méritaient pas d’être trouvés génériquement par script. Cela prend 30 secondes de lecture pour que nous commencions à le voir, 30 autres à expliquer au développeur et, espérons-le, à amender le problème. Encore sous le choc: /
pacifiste le
7
@pacifist Ce n'est pas le genre de style décrit par le répondeur. Le style concerne l'emplacement des accolades, de l'indentation, etc. Si votre développeur junior en utilise trop, vous avez un problème totalement différent de celui de style. Un attribut général de STYLE de codage est qu'il n'affecte pas les performances. Et je pense qu'un nombre important de déclarations if auront un impact sur les performances.
Pimgd
12
Lorsque je fais une critique, je remarque souvent des choses où je pense que "cela ressemble à un bogue", puis j'écris un scénario de test spécifique pour prouver que c'est un bogue. Donc, l’un des nombreux objectifs des revues de code est de trouver des bogues. C'est pourquoi je pense que le point de vue "révision de code ou tests" est un peu trop simple.
Doc Brown
2
En plus de @DocBrown, il existe des cas difficiles à tester - courses de données, certains types de blocages, livelocks, comportements / valeurs non définis (principalement en C / C ++ mais l'ordre des éléments dans les tables de hachage non définies) ou le poireau de ressources (Ouvrir un fichier dans une boucle peut être une mauvaise idée même avec GC). Certaines de ces choses peuvent être détectées par une analyse statique suffisamment intelligente du compilateur .
Commentaires
2
@pacifist, votre exemple spécifique serait totalement détruit lors de la révision du code. Ce serait également un drapeau rouge pour tout analyseur de code statique (complexité cyclomatique 17!). Une revue de code identifierait rapidement cette fonction comme un problème de style sémantique (ou même d'algorithme!). Pourtant. Ce type de problème n’est pas simplement un problème de "style". Si vous le traitez comme tel, vous aurez très bientôt du code vraiment désagréable dans votre référentiel; c'est juste "style", après tout.
Pimgd
12

La chose la plus précieuse que me procure un examen de code est la certitude que le code est clair pour une autre personne. Les variables sont-elles clairement nommées? Le but de chaque morceau de code est-il raisonnablement évident? Est-ce que quelque chose d'ambigu est clarifié avec un commentaire? Les cas limites et les valeurs valides pour les paramètres sont-ils décrits dans les commentaires et vérifiés dans le code?


la source
2
cela ne semble rien offrir de substantiel par rapport aux 4 réponses précédentes
gnat
2
@gnat: Les autres réponses concernent le transfert de connaissances et la détection de bugs. Celles-ci sont importantes, mais la révision du code ne se limite pas à cela.
1
Autant que je sache, cette réponse aborde de manière plus approfondie: "Si vous souhaitez une discussion complète sur les avantages et les stratégies de mise en œuvre des examens par les pairs, je vous recommanderais de consulter Tests par les pairs dans le logiciel: Un guide pratique de Karl Wiegers. " Cette réponse couvre également: "un correctif contre le code trop compliqué"
Gnat
2
@gnat Il souligne un aspect différent des points soulevés dans les autres réponses. Avez-vous déjà été intrigué par votre propre code écrit il y a six mois? La révision du code vous permet d'accélérer ce processus. Si votre code laisse perplexe votre code, vous pouvez toujours le clarifier tant que le problème est encore présent dans votre mémoire.
200_success
1
@ 200_success peut-être. Mais ce point, s'il y est effectivement, semble vraiment mal présenté. Même ton commentaire parvient à le communiquer mieux que cette "réponse". Sans compter que cela ne fait que répéter ce qui a été souligné dans un commentaire précédent qui fait référence à une réponse canonique expliquant cela dans une question connexe
Gnat
7

J'aimerais ajouter deux domaines qui ne sont pas couverts par les autres bonnes réponses:

L’ effet Hawthorne est l’une des bonnes raisons d’examiner les codes: dans notre cas, si vous savez que quelqu'un examinera votre code par la suite, vous aurez beaucoup plus de chances de l’écrire mieux en premier lieu.

Une autre bonne raison est de meilleures pratiques de développement sécurisées. Il suffit de regarder le goto fail d’Apple (une ligne de code dupliquée accidentellement) ou le bogue Heartbleed (un échec fondamental dans la validation des entrées) pour comprendre l’importance de la révision du code dans un cycle de développement sécurisé.

Chris Knight
la source