Grille d'évaluation
Version courante : 2023-09-04
Critères
Placez-vous dans les souliers d’une personne qui n’est pas vous et qui n’a jamais vu le code et qui veut le modifier.
Vous devez agir comme réviseur de code dans l'évaluation et être alerte aux «puanteurs».
Critères binaires
| Critère | Description | Pondération |
|---|---|---|
| B1 | Il est possible de faire un clone du code à partir de la branche principale du dépôt de fichiers (repository). | 2 points |
| B2 | Le code cloné compile sans erreur avec le compilateur indiqué dans les consignes. | 2 points |
| B3 | L'API répond au contrat. Elle est sans bogue. | 2 points |
| B4 | Les tests unitaires passent. | 2 points |
Critère P1
P1) Le code compile sans avertissement
| Critère | Valeur (pts) |
|---|---|
| Aucun avertissement | 4 |
| Moins de 5 avertissements | 2 |
| 5 avertissements ou plus | 0 |
| Pondération: x2 | |
Sauf pour de très rares exceptions à documenter explicitement, il ne devrait pas y avoir d'annotations (ex. : @SuppressWarnings) qui outrepassent les avertissements générés par le compilateur.
Critère P2
P2) Le code est facilement lisible
| Critère | Valeur (pts) |
|---|---|
| La lecture est aisée | 12 |
| La lecture est parfois difficile. Le code a quelques points à améliorer | 8 |
| La lecture demande des efforts. Le code a beaucoup de points à améliorer | 4 |
| La lecture demande beaucoup d’efforts | 0 |
| Pondération: x4 | |
À l'instar de l'écriture littéraire, il n'y a pas d'obstacle à la lecture comme l'incohérence des styles, l'utilisation d'un langage qui n'est pas approprié pour le lecteur [on n'écrit pas en espagnol soutenu pour un public de 3e secondaire francophone!] ou la présence d'intrus dans l'écriture qui surprennent le lecteur et qui le fait débarquer du récit (le parallèle avec le code serait les mots-clés obscurs du langage).
Dans un texte facilement lisible, le lecteur est confortable dans sa lecture. Par exemple, la ponctuation permet de prendre des pauses (une respiration) et fait en sorte que les propos sont agencés (exemple, par l'utilisation d'incise).
Points de vérification suggérés
- Le code est écrit pour la lecture, la révision et la modification par un pair et non pour soi-même.
Indeed, the ratio of time spent reading versus writing is well over 10 to 1. — Robert C. Martin
- Le code respecte le guide de style établit par l’équipe. Par exemple :
- Les balises (ex. :
{ },( ),[ ]) sont toutes alignées de la même façon. - Les blocs de code sont tous indentés de la même façon.
- Il n'y a pas d'espaces superflues.
- Il n'y a pas de sauts de ligne superflus ou manquant (style Kernighan & Ritchie).
- Le nombre de caractères sur une ligne est limité (utilisation judicieuse du ";").
- Si une ligne de code doit avoir un saut de ligne, ce saut est placé au bon endroit.
- Les balises
{ }sont toujours utilisées, même pour les blocs qui ne contiennent qu'une seule ligne. - La casse est cohérente et respecte les conventions en Java (ex. : une constante est écrite en majuscules; les paramètres, variables, propriétés, méthodes, sont en camelCase; le nom des classes sont en PascalCase).
- Les balises (ex. :
- Le code ne comporte pas de passe-passe du cochon qui tousse (variante de la passe du coyote) comme des fonctions lambdas incompréhensibles, des expressions régulières illisibles ou du sel/vinaigre syntaxique obscur amené par le langage.
- D'un autre côté, les principes liés à l'écriture de code en Java sont respectés (ex. : annotations, modificateurs, "littéraux", mots-clés réservés, structures syntaxiques).
- La complexité cyclomatique est raisonnable.
- Les conventions (explicites, par opposition à obscures) sont préférables aux configurations.
- Il n'y a pas de réinvention de la roue en favorisant l'utilisation de librairies.
- Les blocs de code (classes, méthodes, etc.) sont courts.
- Il n’y a pas de “magic values”
- Les commentaires présent sont pertinents et ne sont pas redondant. Les endroits méritant un commentaire en comportent un.
- Il n’y a pas de connaissance dupliquée dans le code (DRY)
- Il n’y a pas de code mort
Critère P3
P3) Le code a du sens (il est simple, élégant, localisable et sans doublon)
| Critère | Valeur (pts) |
|---|---|
| J’ai compris le code à la première lecture : intention, comportement, comment il fonctionne | 12 |
| Je devais parfois revenir en arrière pour mieux comprendre -ou- je suis assez confiant de modifier le code mais j’aimerais avoir un réviseur ou poser une question | 8 |
| Je me suis parfois arrêter pour réfléchir -ou- je me suis promené un peu partout dans le code pour le comprendre -ou- je quelques craintes à modifier le code seul | 4 |
| Je n’ai pas le sentiment d’avoir tout compris -ou- je n’ai pas totalement confiance de modifier le code sans le mettre en péril ou sans créer des effets de bord | 0 |
| Pondération: x4 | |
Comme pour la rédaction d'un texte, le code a une certaine fluidité ou continuité : passer doucement d'une phrase, d'un paragraphe ou d'une page à l'autre sans que les concepts se fracturent. Cette fluidité implique aussi :
- l’utilisation du bon vocabulaire,
- l’utilisation judicieuse de tropes/figures de style (ex. : métaphore, euphémisme/hyperbole, litote) afin de mieux passer un message
- d'éviter les répétitions inutiles,
- être cohérent dans ce qui est exprimé.
Points de vérification suggérés
- Les noms de classes, méthodes, propriétés, variables, etc. sont significatifs.
- Le choix des types est judicieux (ex. : utiliser un entier au lieu d'un nombre à virgule flottante, utiliser un entier au lieu d'une chaîne de caractères).
- Le code est suffisamment explicite pour ne pas nécessiter de commentaires.
- Le code est à l’endroit où on s’y attend naturellement. Il n’y pas de surprise ni de recherche à faire pour trouver un morceau de code. Par exemple, les propriétés d’une classe arrivent avant les méthodes.
- Même si le code est en Java, il respecte la philosophie Zen of Python.
- Le code ne contient pas d'instructions inutiles.
- Du code qui n'est jamais exécuté (ex. : conditions ou blocs de code qui ne seront jamais exécutées).
- Cela inclut les commentaires et du code commenté.
- Il n'y a pas de code superflu pour se préparer à un futur que l'on ne connait pas (YAGNI).
- Le code ne se répète pas (DRY).
- Les points de sortie sont facilement identifiables. Le flux est cohérent.
- Il n'y a pas de retour obscur que l'appelant doit tester (ex : retourner
null). - Les exceptions sont utilisées de façon judicieuses et descriptives (leurs noms correspondent à ce qu'elles lèvent).
- Les exceptions explicites sont attrapées spécifiquement (par opposition à un CatchAll).
- Les exceptions sont emballées (wrapped-up) quand elles doivent remonter.
- Il n'y a pas de retour obscur que l'appelant doit tester (ex : retourner
- Rien n'est codé en dur.
Critère P4
P4) Le code respecte le paradigme OO
| Critère | Valeur (pts) |
|---|---|
| La structure du code respecte le paradigme OO | 9 |
| Certains éléments devraient être améliorés | 6 |
| Le paradigme OO (héritage, polymorphisme, etc.) sont difficiles à identifier | 3 |
| Le code est un gros bloc solide -ou- un plat de spaghettis cuits -ou- principalement procédural | 0 |
| Pondération: x3 | |
Un texte a une structure logique selon le message que l’on essaie d’exprimer. Par exemple, on s'attend d'avoir le sujet amené, le sujet posé et le sujet divisé dans un texte explicatif. Par ailleurs, toujours dans un texte explicatif, on commence que très rarement par la conclusion. Certaines structures attendues aident aussi à la lecture et à la compréhension : un texte est composé de mot dans des phrases à l'intérieur de paragraphes sectionnés en chapitre cohérents en fonction des protagonistes, de l'action, du temps, des situations, etc.
Points de vérification suggérés
- Tout en gardant une intégration/cohésion forte du côté du domaine (voir le critère P8), le couplage est faible entre les composantes.
- Les dépendances sont stables.
- Il n'y a pas de couplage entre les méthodes d'une même classe.
- L’injection de dépendances est favorisée (y compris par des constructeurs).
- Les Factory sont utilisées de façon judicieuse.
- Autres principes liés à GRASP
- Bonne délégation des responsabilités en fonction de l’information, principe du spécialiste d’information (une des applications du SRP)
- Indirection : le contrôleur contrôle : il ne contient pas de logique ni de représentation des données (vues) ni le modèle (MVC)
- Les variations par l’héritage sont protégées (super ensemble de l’OCP),
- Les services (pure invention en GRASP) sont sans état, ne sont pas reliés à un objet (entité ou objet-valeur) et ont des interfaces qui sont définies selon les autres éléments du domaine (voir le critère P8).
- La structure du code passe des messages. Il n’est pas procédural.
- Les classes/membres statiques sont utilisés de façon judicieuse.
- La classe et son domaine ne sont pas anémiques.
- Les interfaces sont utilisées dans un désir d'abstraction afin de définir un contrat.
- L'implémentation est différenciée de son abstraction (y compris dans son nom).
- Le code préfère la composition à l'abstraction. Les abstractions ne sont pas utilisées seulement pour réutiliser du code.
- Les abstractions ne sont pas court-circuités (e.g. utilisé directement une classe enfant quand ça ne devrait pas être le cas)
- Il n’y a pas d’obsession de la primitive
- Il n’y a pas de condition du style
if (... instanceof ...) - Il n’y a pas de couplage temporel
- Le design permet facilement d’injecter des mocks
- Il n’y a pas de dépendances cycliques (entre les packages ou les classes)
Critère P5
P5) Le code est écrit et structuré pour être facilement testable unitairement et réusinable (SOLID!)
| Critère | Valeur (pts) |
|---|---|
| Le code est facilement testable pour l'entièreté des comportements attendus ‑et- il est facilement réusinable sans changer les tests | 12 |
| Certaines classes/méthodes doivent être découpées autrement pour qu'il soit testable unitairement dans son entièreté | 8 |
| Beaucoup de classes/méthodes doivent être découpées autrement pour qu'il soit testable unitairement dans son entièreté | 4 |
| Il est très difficile de faire des tests unitaires automatisés | 0 |
| Pondération: x4 | |
Points de vérification suggérés
- Respect des principes SOLID
- SRP: Les classes n’ont qu’une seule raison de changer
- SRP: Les méthodes n’ont qu’une seule raison de changer (attention à ne pas mélanger “query” et “command”)
- OCP: Si j’ajoute une implémentation à une interface, aucun autre bout de code ne devrait changer (à part potentiellement à la création, via une factory par exemple)
- OCP: Il n’y a pas d’abstraction court-circuitée, via un instanceof ou des méthodes du type
animal.estUnChien() - LSP: Les abstractions dans le domaine sont calquées sur les besoins d’affaires
- LSP: Les abstractions ne sont pas utilisées seulement pour réutiliser du code
- LSP: Aucune sous-classe dans une hiérarchie ne lance d’exception de type “not implemented”
- ISP: Dans la mesure du raisonnable, les clients des interfaces utilisent toutes les méthodes de l’interface
- DIP: Le sens des flèches est bon et toutes pointent vers l’intérieur de l’hexagone
- DIP: Les collaborateurs d’une classe sont au même niveau, et ont le même cycle de vie
- DIP: Les repositories sont utilisés correctement et les classes sont dans la bonne couche
- Respect du Tell, don't ask
- Utilisation du polymorphisme au lieu de types/classes/propriétés contenant des conditions
- Le modèle de classe est «orthodoxe», c'est-à-dire qu'il est représentable simplement.
- Il est facile de réusiner le code sans toucher aux tests.
- Les DTO sont placés au bon endroit pour respecter le sens des flèches
- S’il y a une couche de services applicatifs, ceux-ci ne font qu’orchestrer le domaine
Critère P6
P6) Les tests unitaires sont descriptifs et testent ce qu’ils sont supposés tester
| Critère | Valeur (pts) |
|---|---|
| Tous les tests unitaires sont bien décrits (nommés), ils testent l'ensemble des comportements souhaités et il n'y en pas de superflus | 9 |
| Il manque quelques tests -ou- quelques comportements restent à tester -ou- au moins un test a un problème de nommage -ou- la structure des tests pourraient être améliorés | 6 |
| Les tests sont trop lourds -ou- le choix des doublures (mock) n'est pas le bon dans quelques cas -ou- les tests ne couvrent qu'une partie des comportements attendus | 3 |
| Il manque plusieurs tests -ou- il y a régulièrement une utilisation incorrecte de doublures -ou- de façon générale, les tests sont difficiles à lire, à comprendre ou ne reflètent pas ce qu’ils doivent tester | 0 |
| Pondération: x3 | |
On s'attend à voir une correspondance forte entre les tests, le domaine, les comportements souhaités et le code produit. Les tests influencent le code et non l'inverse, c.-à-d. que le découpage du code ne doit pas influencer la structure des tests: TDD.
Points de vérification suggérés
- La nomenclature des tests est évidente (Ex. :
Arrange-Act-Assert,Given-When-Then, etc). - Les tests ont tous des noms significatifs : Son nom démontre toujours le comportement attendu et l’action, ainsi que le contexte lorsque pertinent. Il n’y a pas de bruit inutile dans le nom du test.
- Une classe de test ne teste qu’une seule unité (généralement une seule classe)
- Chaque cas de test ne teste qu'un seul comportement à la fois (≠ test par méthode).
- Il n’y a pas de cas de test inutiles (cas impossibles ou tests en double).
- Tous les tests sont exécutés.
- Le choix des doublures (dummy, fake, stub, spy, mock) est judicieux.
- L'ensemble de tests couvrent tous les comportements (≠ couverture de code).
- Les tests sont courts, les méthodes privées ou autres classes sont utilisés afin de rendre les tests plus lisibles. Il n’y a pas de bruit inutile.
- Chaque méthode de test est bien divisée, il est facile de différencier les parties du test
- Il ne devrait pas y avoir d'instructions de contrôle dans les tests (
for/while/if/switch/catch/throw). - Il n’y a pas de surutilisation des mocks (e.g. pour des structures de données, le framework, etc)
- Il n’y a pas de test sans assertions, ou de tests qui ne peuvent pas échouer.
- Le nom de la méthode de test et son contenu sont cohérents
- Pas de “magic values”
- Les tests sont faciles à lire, on pourrait presque les lire tel quel pour raconter une histoire!
- Les tests sont uniformes dans l’application, il n’y a pas de style différent selon l’auteur
Critère P7
P7) Le code est facile à faire évoluer et à maintenir
| Critère | Valeur (pts) |
|---|---|
| Il est trivial de modifier le code | 12 |
| Un effet d’avalanche sera présent mais touche peu d’autres classes et uniquement des classes “proches” | 8 |
| La modification du code implique de modifier plusieurs classes externes ou des classes éloignées logiquement | 4 |
| Une grande partie du code doit être jeté pour faire des modifications -ou- au moins un test doit être modifié pour une action de réusinage à l’intérieur d’un membre d’une classe | 0 |
| Pondération: x4 | |
Pour évaluer ce critère, imaginez une modification potentielle à faire dans le code. La modification ne devrait impliquer que la ou les classes impactée.s logiquement, sans avoir d’effet d’avalanche en dehors de celles-ci.
Points de vérification suggérés
- Le choix des modèles d'architecture permet de faire face aux changements de la part de l'utilisateur (ou du client dans le cas d'une API).
- Sans toucher aux tests, il est facile de :
- faire évoluer le modèle de classes en fonction de nouveaux besoins,
- d'ajouter/d'enlever (😱) des méthodes et des propriétés sans briser l'interface,
- repérer les bogues fonctionnels,
- respecter les exigences non fonctionnelles.
- Changer l’implémentation d’une classe
- Pour un changement donné, tous les membres de l’équipe peuvent rapidement identifié les endroits qui seront à modifier
- Le design est simple et facile à comprendre (ou à expliquer)
- Il serait techniquement simple de modifier la persistence des données (e.g. changer de BD)
Critère P8
P8) L’architecture et le code correspond au domaine et est cohérent
| Critère | Valeur (pts) |
|---|---|
| L’architecture et le code reflètent le domaine dans sa précision, la rigueur (précision) de son découpage et sa cohérence et son adaptabilité/flexibilité | 9 |
| Le découpage montre certaines faiblesses facilement corrigibles quant à son adéquation avec l’interprétation sensée du domaine | 6 |
| Les contextes, même s’ils sont présents, ne sont pas délimités logiquement par rapport au domaine -ou- les contextes sont délimités à des niveaux de granularité différents -ou- le code est en partie incohérent dans l’expression du langage du domaine | 3 |
| L’expression du domaine par le code est généralement anémique -ou- le découpage ne représente pas les contextes délimités du domaine -ou- l’architecture et le code ne correspond pas au domaine -ou- le nommage n’exprime pas le langage utilisé par le domaine | 0 |
| Pondération: x3 | |
Le code et son architecture (découpage) représente le domaine, c’est-à-dire, qu’il correspond aux besoins exprimés, aux entités du domaine, aux comportements souhaités et aux objectifs d’affaires, autant par le découpage des classes que par l’utilisation d’un langage ubiquitaire propre au domaine.
Points de vérification suggérés
- Les abstractions représentent celles du domaine.
- Le découpage est une réflexion des contextes (bounded contexts). En d’autres mots, les contextes du domaine sont bien délimités.
- Le découpage des composantes du domaine respecte un modèle hexagonal.
- La carte contextuelle (la position et la relation entre les contextes) traduit fidèlement les modèles mis en contextes délimités.
- Les concepts (blocs de construction) du domaine sont explicites. Par exemple, ils ne sont pas dissimulés à l’intérieur de méthodes ou de propriétés.
- Le lien entre les agrégats est fait par valeur (et non par référence)
- Les opérations sont toujours faites via la tête d’aggrégat.
- Pour chaque opération, l’aggrégat au complet est chargé en mémoire, et celui-ci est sauvegardé au complet à la fin (sauf s’il y a un mécanisme de “lazy loading”)
- Les identifiants naturels des classes du domaine sont clairement identifiés et utilisés le plus possible
- Les entités ne se substituent pas aux objets-valeur.
- Les primitives n’ont pas une signification sémantique riche (obsession de la primitive).
- Les associations permettent aux programmeurs de découvrir les relations (≠ dépendances) entre les entités. Il est facile de traverser un repository afin de découvrir les relations entre les entités : le modèle. L’implémentation du client peut ignorer les détails d’implémentation du repository.
- Il y a une séparation/isolation/découplage de la logique d'affaires, de l'interface (graphique ou applicative) et de l'accès aux données et autres DTO. Par exemple, il ne devrait pas avoir du code SQL dans l'interface!
- Il est facile de réusiner le code sans toucher à son découpage, si le domaine ne change pas.
- Le code est riche (il n’est pas découpé de façon procédurale, c’est-à-dire anémique). Les propriétés et les méthodes agissant sur ces propriétés sont encapsulées ensembles.
- Le nommage des variables, des paramètres, des classes et ses membres utilisent un langage ubiquitaire cohérent qui correspond en tout point à celui du domaine exprimé par le client.
Critère P9
P9) Le code reflète les pratiques de programmation sécuritaire
| Critère | Valeur (pts) |
|---|---|
| Le code est réputé comme sécuritaire et risque peu d’introduire une erreur involontairement | 4 |
| Le code est écrit de manière généralement robuste et sécuritaire. Mais il est probable d’introduire une erreur par accident | 2 |
| Un analyseur statique de vulnérabilité peut trouver une erreur -ou- la probabilité d’introduire par accident du code comportant des bogues, des effets de bord ou de nouveaux vecteurs d’attaques est assez probable | 0 |
| Pondération: x2 | |
Du code sécuritaire est réputé comme du code qu’il est possible de modifier sans danger d’introduire des bogues, des effets de bord ou de nouveaux vecteurs d’attaque. Il ne s’agit pas uniquement des attaques de cybersécurité mais aussi les modification par accident d’un autre développeur qui peut briser le code sans le vouloir.
Points de vérification suggérés
- Les validations nécessaires sont réalisées (ex. : vérification des
null, validation de tous les paramètres, validation de toutes les données passant par une interface externe/API). - Les validations sont faites aux bons moments (pas trop tôt ni trop tard).
- Il n'y a pas d'injection de code possible.
- Il n’y a pas de risque de blocage (deadlock) ou de famine (starvation). Il n’y a pas de passe-passe pour les éviter (ex. : attentes arbitraires).
- Il n’y a pas de dépendances temporelles.
- Les noms de méthodes ne portent pas à confusion, il n’y a pas d’ambiguïté dans l’intention.
- Il n’y a pas de commentaire disant comment ne pas utiliser une méthode. La méthode se protège elle-même contre son utilisation inadéquate.
- Les exceptions sont gérées adéquatement. Il n’y a pas de fuite d’information via le UI, et il n’y a pas de possibilité de comportement inattendu ou de corruption de données en cas d’exception.