Sept signes d’alerte lors de l’évaluation qui prédisent des modifications coûteuses à venir

La plupart des codes problématiques ne semblent pas défectueux. Ils se compilent, passent leurs tests et fonctionnent bien dans une démonstration, mais les reviewers expérimentés parviennent souvent en une minute à deviner que la prochaine modification, le prochain incident ou le prochain transfert de responsabilités causera des problèmes. Ce guide présente sept signaux auxquels ils réagissent, explique ce que chacun prédit, et montre comment distinguer un véritable problème d’un schéma qui semble simplement suspect, afin que vous puissiez appliquer le même jugement à vos propres analyses.

Ce que révèle vraiment un « code smell »

Un critique expérimenté n’a que rarement besoin de lire un module entier pour détecter des problèmes. Un argument booléen dont la finalité reste invisible là où la fonction est appelée, une utilité qui a acquis une douzaine de comportements optionnels, un catch qui transforme chaque échec en la même valeur d’apparence innocente : rien de tout cela ne provoque de panne aujourd’hui. Le problème réside dans ce qu’ils pourront exiger demain.

Cet instinct provient de l’expérience plutôt que d’un guide de style plus strict. Ceux qui ont maintenu des systèmes pendant des années ont vu une règle simple se transformer en trois versions contradictoires, une abstraction pratique devenir le point d’arrivée de tous les cas particuliers, et un mécanisme par défaut masquer une panne en production pendant des semaines. Ce qu’ils ont appris à repérer, c’est là où un morceau de code accumule de l’incertitude.

Il est utile de considérer une odeur comme un indice, et non comme un jugement. Cela ne prouve pas que la mise en œuvre est erronée ; cela indique simplement que le code mérite un examen plus approfondi :

• Quelle personne ou quel module est responsable de cette décision ?
• Comment cela se comportera-t-il lorsque les exigences changeront ?
• Un échec restera-t-il visible lorsqu’il se produira ?
• Le développeur suivant pourra-t-il comprendre pourquoi le code est conçu de cette manière sans avoir à retracer celui qui l’a écrit ?

Les sept signaux ci-dessous sont ceux qui se transforment le plus souvent en coûts si personne ne pose ces questions tôt.

1. Une fonction effectuant plusieurs tâches non liées

Les fonctions longues font l’objet de critiques, mais leur longueur n’est que rarement ce qui inquiète un réviseur expérimenté. Une fonction peut compter de nombreuses lignes car elle effectue une opération claire en séquence de pas lisibles. Une autre peut tenir en vingt lignes et rester difficile à utiliser, car elle gère en même temps la validation des entrées, les vérifications de permissions, l’enregistrement dans la base de données, la mise en forme de la réponse, le déclenchement de notifications et la traduction des erreurs.

Le véritable problème est que des responsabilités mélangées donnent à une fonction de nombreuses raisons indépendantes de changer. Une nouvelle règle d’accès l’affecte, un changement dans le format de la réponse l’affecte également, une migration de schéma l’affecte, ainsi qu’une nouvelle exigence en matière de notifications. Des préoccupations qui devraient évoluer séparément finissent par se rencontrer en un seul endroit, et même une petite modification oblige à comprendre l’ensemble du flux.

processOrder pourrait vérifier l’identité du client, calculer les prix, gérer les stocks, créer une intention de paiement, enregistrer la commande et envoyer un e-mail de confirmation. Le nom reste vague car un nom précis nécessiterait plusieurs paragraphes pour être complet.

Pourquoi l’extraction mécanique n’est pas la solution

Décomposer la fonction en parties plus petites ne suffit pas automatiquement. Extraire des segments en se basant sur le nombre de lignes peut disperser la même confusion dans davantage de fichiers, tout en laissant subsister toutes les dépendances cachées. Une meilleure approche consiste à rechercher des limites qui ont un sens concret :

• Où est prise une décision commerciale ?
• Quelle partie a un effet secondaire sur un système externe ?
• Quelle partie correspond à une simple transformation de données ?
• Quelle partie gère la transaction ?

Dès que ces coutures deviennent visibles, le code peut être organisé selon le fonctionnement réel de l’opération. Le problème fondamental n’a jamais été la quantité de travail effectuée par la fonction, mais plutôt le fait que personne ne pouvait modifier une seule décision sans avoir en même temps à garder à l’esprit plusieurs autres décisions non liées.

2. Paramètres booléens qui activent des modes cachés

await updateAccount(accountId, true, false);

Pour le comprendre, il faut ouvrir l’implémentation et déterminer si le premier true force la mise à jour, déclenche une notification, contourne la validation, conserve l’historique ou active le compte. Celui qui a écrit la fonction s’en souvient peut-être, mais tous les autres doivent reconstituer son sens à partir des positions des arguments, et un échange de paires de littéraux permettra de passer tous les contrôles de type.

Combinaisons jamais conçues

Le problème s’aggrave à mesure que les flags s’accumulent. Trois booléens donnent huit combinaisons possibles, dont plusieurs n’ont jamais été envisagées. Si une fonction accepte forceUpdate, skipValidation et sendNotification, que se passe-t-il lorsque la mise à jour forcée contourne la validation mais envoie quand même une notification ? Le système de types autorise volontiers cette appelation, même si ce n’est pas souhaité dans le domaine métier.

Les examinateurs signalent cela car un ensemble de drapeaux indique généralement qu’une seule fonction remplace plusieurs opérations. Elles peuvent partager des éléments internes, mais pas leur finalité. Modifier les coordonnées de contact, vérifier un compte et appliquer une modification administrative peuvent toutes écrire dans la même ligne, mais aucun appelant ne devrait avoir besoin de se souvenir quel schéma de drapeaux sélectionne quel flux de travail.

Choisir la refacturation appropriée

Dans certains cas, il suffit de passer à un objet à un seul argument dont les propriétés ont des noms, car updateAccount(id, { notify: true }) s’explique au moins à l’endroit où il est appelé. Dans d’autres cas, des fonctions distinctes, telles que verifyAccount et overrideAccount, rendent l’intention métier explicite et éliminent complètement les combinaisons impossibles. Le choix de la méthode dépend du fait que les comportements soient des variations réelles d’une seule opération ou des opérations distinctes qui se retrouvent à partager du code.

Les booléens ne sont pas intrinsèquement mauvais. Une option clairement nommée comme includeArchived est tout à fait raisonnable. Le signe d’alerte, c’est un flag qui change le sens de la fonction plutôt que de modifier un détail déjà bien compris.

3. La même règle métier exprimée sous différentes formes

Le texte copié-collé est facile à repérer. Les connaissances dupliquées sont plus dangereuses, car chaque copie peut paraître complètement différente.

Prenons par exemple une règle de permission : elle peut apparaître sous forme d’un bouton caché dans l’interface utilisateur, d’une vérification de rôle dans un contrôleur, d’une condition de statut dans un service ou d’une clause WHERE dans une requête. Chaque couche semble protéger le système, mais chacune peut encodifier une conception légèrement différente de ceux qui ont le droit d’agir.

Tout fonctionne correctement jusqu’à ce que la politique change. Un point d’entrée prend en compte la nouvelle règle, un autre conserve la logique ancienne, tandis qu’une tâche planifiée contourne les deux en se connectant directement à un service interne. L’interface propose alors une action que le serveur refuse. Une partie de l’application considère qu’un utilisateur est autorisé en raison de son rôle, tandis qu’une autre insiste sur le fait qu’il doit posséder l’enregistrement en question.

Les évaluateurs expérimentés repèrent des combinaisons récurrentes de statuts, de rôles, de drapeaux et de dates, car de telles conditions représentent souvent la même signification métier écrite plus d’une fois. Ils se demandent où se trouve la décision officielle, et si les autres couches appliquent cette décision ou la redéfinissent discrètement.

Ces couches ayant des objectifs différents sont acceptables

Cela ne signifie pas qu’une règle ne puisse apparaître que dans une seule ligne de code dans tout le système. L’interface utilisateur peut refléter l’autorisation afin d’améliorer l’utilisabilité, tandis que le backend doit la faire respecter pour des raisons de sécurité. Une contrainte de base de données peut protéger la même invariante au niveau du stockage. Ces couches ont des tâches distinctes. Le problème apparaît lorsque chacune répond à la question métier selon ses propres critères.

Un design sain attribue à la règle un unique responsable délibéré et permet aux autres composants de consommer, d’appliquer ou d’afficher le résultat. Lorsque la responsabilité n’est pas claire, chaque modification future de la politique se transforme en une quête : il faut retrouver chaque endroit où la règle est codée discrètement et être sûr d’en avoir trouvé tous. Toute personne ayant vécu des évolutions successives de politiques sait que le renouvellement constant des connaissances coûte généralement plus cher que la modification répétée du code.

4. Un outil partagé avec un objet d’options en expansion

De nombreuses abstractions problématiques commencent leur existence en tant qu’outils utiles. Lorsque deux endroits utilisent une logique similaire, la partie commune est extraite, ce qui donne un résultat plus compact, ciblé et facile à comprendre.

Alors les deux appelsants commencent à vouloir des choses différentes. L’un a besoin de tentatives répétées ; l’autre ne doit jamais répéter sa demande. L’un bénéficie du cache ; l’autre ne doit jamais voir de valeurs obsolètes. L’un renvoie un en-tête complet ; l’autre transmet les données par flux continu. Plutôt que de séparer ces workflows, l’outil crée une nouvelle option pour chaque différence.

Bientôt, l’objet des options couvre la traduction des erreurs, la transformation des résultats, les paramètres de cache, le nombre de tentatives, la gestion de l’authentification, les fonctions d’ancrage ainsi que quelques interrupteurs booléens. La plupart des appelsants utilisent deux ou trois d’entre eux, mais toute modification risque d’affecter tous.

Ce n’est qu’une illusion, car l’abstraction a cessé d’éliminer la complexité pour commencer à la concentrer. Les points d’appel semblent simples, mais comprendre le système aujourd’hui signifie savoir comment chaque option interagit avec toutes les autres. Un grand nombre d’appels n’est pas une preuve que l’abstraction est solide ; parfois, cela montre simplement que l’aide fournie est devenue inévitable.

Syntaxe similaire versus comportement partagé

La question clé est de savoir si les points d’appel présentent un comportement stable ou simplement du code à l’aspect similaire. Deux opérations peuvent toutes deux envoyer des requêtes HTTP, mais gérer les échecs et les tentatives de réessai de manières opposées. Deux formulaires peuvent avoir les mêmes champs, mais des mécanismes de validation, d’envoi et de gestion des brouillons différents. Les fusionner élimine la duplication visible tout en rendant plus difficile l’expression de chaque différence réelle.

Ce qui fonctionne mieux est généralement une abstraction plus restreinte introduite ultérieurement. Au lieu de partager l’ensemble du flux de travail, partagez une seule composante stable, telle qu’une transformation, une politique ou une appel de bas niveau, et maintenez les flux de travail distincts de manière explicite. La réutilisation reste précieuse ; ce qui devrait susciter des inquiétudes, c’est un schéma où chaque nouvelle exigence ajoute une autre option dont les utilisateurs existants n’ont ni besoin ni compréhension. Pour une perspective axée sur le frontend concernant ce même problème, consultez pourquoi les abstractions frontend se transforment silencieusement en dette technique.

5. Gestion des erreurs qui dissimule l’échec en succès

Un bloc catch peut faire paraître une application solide tout en érodant discrètement la confiance qu’on lui porte.

Imaginez une couche d’accès aux données qui répond à une erreur de base de données en renvoyant []. Ou un service dont l’appel HTTP échoue et qui renvoie simplement null. L’interface utilisateur associe chaque demande rejetée à un état vide. Rien ne plante, mais le système ne peut plus distinguer une absence légitime de données d’une opération qui a échoué.

Les auditeurs repèrent rapidement ce problème car ils connaissent son impact lors d’un incident. Un tableau de bord indique zéro enregistrement lorsque la base de données est hors service. Un client voit « Aucune commande trouvée » après l’expiration silencieuse de sa session. Une tâche en arrière-plan marque un élément comme terminé parce qu’une dépendance n’a rien renvoyé d’utilisable. L’interface reste calme car elle a remplacé la vérité par une réponse crédible.

Questions à se poser pour chaque bloc de gestion des erreurs

Lors de l’analyse du traitement des erreurs, vérifiez trois éléments :

• La cause initiale est-elle conservée, par exemple en tant que cause d’une nouvelle erreur, ou au moins dans les journaux ?
• Le appelant peut-il distinguer un résultat métier attendu d’une panne technique ?
• La couche qui capture l’erreur détient-elle réellement la responsabilité de la décision suivante ?

Lorsque le code à ce niveau ne dispose d’aucun moyen de se remettre en état, de réessayer, d’annuler des actions partielles ou de proposer un substitut approprié, capturer l’erreur ne sert à rien d’autre qu’à détruire les preuves.

Rien de tout cela ne signifie pour autant que les exceptions brutes doivent parvenir aux utilisateurs. Les messages affichés aux personnes doivent être sûrs et compréhensibles, et les erreurs internes nécessitent souvent d’être traduites en catégories de domaine stables. L’exigence est que cette traduction conserve suffisamment de sens pour permettre à l’application et aux développeurs de réagir correctement.

Un résultat vide doit indiquer que l’opération a réussi sans trouver rien. Une opération échouée doit rester une erreur, sauf si le système a défini intentionnellement un mécanisme de secours fiable. Fusionner les deux par commodité ralentit le débogage et fournit aux utilisateurs des informations trompeuses. Une bonne gestion des erreurs les expose ; une mauvaise gestion les cache.

6. Un petit requis avec des différences importantes et non liées

Les reviewers expérimentés examinent souvent la structure d’une demande de fusion avant de lire les lignes individuelles. Lorsqu’un requis modeste s’étend sur une longue liste de fichiers non liés, cela indique généralement un couplage caché ou une tâche qui a dévié de son objectif initial.

L’ajout d’un seul champ peut légitimement avoir des répercussions sur la persistance des données, les services, les réponses API et l’interface utilisateur ; il s’agit d’un changement cohérent à travers les différentes couches du système. Le problème apparaît lorsque la même demande de modification réécrit également des utilitaires partagés, renomme des types non liés, modifie le traitement des erreurs, déplace des modules et met à jour de gros fichiers d’aperçu.

Chacune de ces modifications peut représenter une amélioration, mais prises ensemble, elles regroupent plusieurs raisons de changement en une seule unité. Les examinateurs doivent évaluer simultanément la nouvelle fonctionnalité, la refonte, le changement de contrat et les opérations de nettoyage. Si une régression échappe à l’attention, il devient plus difficile de déterminer quelle modification en est responsable. Si la fonctionnalité doit être annulée, les améliorations non liées sont également révoquées.

Pourquoi le codage assisté par l’IA rend ce phénomène plus courant

Les outils de génération de code rendent plus facile la chute dans ce schéma, car une réécriture complète coûte presque rien à réaliser. Si vous demandez la correction d’une seule erreur, vous pouvez recevoir une fonction de remplacement parfaitement affinée, avec de nouveaux noms, un flux de contrôle différent, des abstractions fraîches et un comportement en cas d’erreur modifié. Sa création a coûté presque rien ; en revanche, sa vérification exige toujours de l’équipe le même effort d’examen qu’auparavant.

Une bonne règle consiste à garder l’ampleur d’une modification en proportion de la clarté avec laquelle elle peut être expliquée. Un diagnostic précis mérite généralement une solution ciblée, tandis que des corrections plus étendues peuvent faire l’objet d’une demande de pull request distincte, où elles pourront être examinées et testées en fonction de leurs propres mérites. Il ne s’agit pas d’exiger des pull requests minuscules partout ; certaines tâches architecturales sont nécessairement de portée large. La question est de savoir si chaque modification significative vise un résultat cohérent. Lorsque la différence entre l’état initial et le nouvel état est plus importante que le problème lui-même, plusieurs problèmes sont généralement traités en même temps sans être séparés, ce qui rend plus difficile l’identification des causes et des effets.

7. Du code qui fonctionne mais résiste à la vérification

Le problème est le plus aigu dans les flux asynchrones à plusieurs étapes. Imaginez une fonction qui enregistre un changement, émet un événement de domaine, notifie un utilisateur et ajoute une ligne au journal d’audit. Si la troisième étape échoue, un test peut-il déterminer quelles étapes ont déjà été terminées ? Peut-on forcer une dépendance à expirer ? Peut-on simuler l’exécution de la fonction deux fois ? Peut-on reproduire une mise à jour obsolète sans compter sur la chance liée aux délais ?

Un code facile à écrire mais difficile à vérifier favorise les tests manuels. Quelqu’un suit le parcours réussi, voit le bon résultat et en conclut que la fonctionnalité est terminée. En production, cependant, apparaissent la concurrence, les tentatives de réessai, des données inhabituelles et des opérations inachevées que personne n’a testées localement.

Vérifier au niveau où réside le risque

La pureté partout et les mocks pour tout ne constituent pas non plus la solution. Les tests basés principalement sur des mocks ont tendance à figer les détails internes sans tenir compte du comportement réel de la base de données, du réseau ou des transactions. Une approche plus efficace consiste à déterminer quelles garanties sont importantes et à vérifier chacune d’elles là où réside réellement son risque :

• Les transformations pures peuvent être testées unitairement de manière isolée.
• Les invariants de la base de données nécessitent souvent des tests d’intégration sur une base de données réelle.
• Les frontières des services peuvent exiger des tests de contrat.
• Le comportement distribué peut dépendre de journaux structurés et d’ID de corrélation, car toutes les conditions de production ne peuvent pas être reproduites fidèlement.

Injecter une horloge ou une dépendance, au lieu de recourir aux variables globales, est souvent le moindre changement qui permet de rendre les décisions importantes contrôlables. Le problème ne réside pas tant dans un faible taux de couverture que dans du code dont les décisions clés ne peuvent être contrôlées, observées ou expliquées une fois que le parcours idéal cesse de fonctionner correctement.

Points clés

Aucun de ces signaux ne constitue un verdict automatique. Une fonction longue peut représenter une seule opération bien définie, une option booléenne peut être entièrement évidente, un outil partagé peut refléter un comportement véritablement stable, et un changement important peut être inévitable. Considérez cette liste comme un moyen de déterminer où creuser davantage, et non comme une liste à cocher à remplir mécaniquement.

Lors de l’examen, les questions fondamentales sont les suivantes :

• Les responsabilités sont-elles mélangées, au point qu’un seul changement entraîne avec lui des décisions non liées ?
• Les appelsants peuvent-ils comprendre une fonction sans devoir décoder des indicateurs positionnels ?

Chaque règle métier a-t-elle un responsable clairement défini ?

Une abstraction partagée cache-t-elle des différences réelles derrière les options ?

Gère-t-on correctement les erreurs afin de conserver la vérité sur ce qui a échoué ?

La différence est-elle proportionnelle à l’exigence qu’elle répond ?

Le comportement important peut-il être vérifié en cas de problème ?

Ce que les reviewers expérimentés détectent réellement, c’est l’incertitude différée : la possibilité que la prochaine exigence oblige quelqu’un à comprendre une logique non liée, que le prochain incident laisse trop peu d’indices pour enquêter, que le développeur suivant ne sache pas quel comportement était intentionnel, ou que l’appelant suivant ajoute encore une option à une abstraction déjà surchargée. Ils reconnaissent rapidement ces signaux d’alerte, car ils en ont déjà payé le prix : débrouiller des helpers surchargés, tracer les exceptions ignorées, concilier des règles contradictoires et examiner la fonctionnalité « petite » qui a touché la moitié du codebase. L’objectif n’est pas d’éliminer le code qui semble désordonné, mais de repérer celui qui rendra le travail futur plus difficile qu’il ne le devrait.

Lectures complémentaires