Ne simplifiez pas ce code

• Le fichier pv_controller.go de Kubernetes est le contrôleur qui synchronise le binding PV/PVC et précise dès le haut du fichier qu’il ne faut pas le « simplifier » et qu’il faut conserver le style space shuttle
• Ce style consiste à associer un else à chaque if et à laisser des commentaires même sur des conditions qui paraissent évidentes, afin de rendre visibles dans le code les branches examinées et les intentions
• Au cœur de la conception se trouve un pointeur bidirectionnel reliant pvc.Spec.VolumeName et pv.Spec.ClaimRef, ce qui permet de gérer et de récupérer les situations de concurrence, suppression, modification utilisateur et binding simultané dans un environnement sans transactions
• Le contrôleur combine surveillance des changements PV/PVC, cache interne, file d’attente mono-worker, enregistrement d’événements, provisioning dynamique et interface de migration CSI pour gérer les transitions d’état du binding
• Les branches verbeuses et les commentaires servent à préserver dans le code la connaissance métier du comportement et le contexte de reprise après échec ; les modifications futures doivent donc suivre le même style

---

Rôle et principes de rédaction de pv_controller.go

• pv_controller.go est le fichier d’implémentation du PersistentVolumeController dans le package Kubernetes persistentvolume
• Ce contrôleur aligne l’état de PersistentVolumeClaim et de PersistentVolume
  • contrôleur de cache qui surveille les changements de PersistentVolume
  • contrôleur de cache qui surveille les changements de PersistentVolumeClaim
  • synchronisation de l’état PV/PVC à partir des événements de changement des deux objets
• Le commentaire en tête du fichier avertit à plusieurs reprises de ne pas simplifier ce code
  • le nom du style est space shuttle style
  • il consiste à mettre un else correspondant à chaque instruction if
  • l’objectif est de rendre explicites toutes les branches, sauf les simples vérifications d’erreur
  • même les comportements qui semblent évidents sont documentés par des commentaires pour permettre aux mainteneurs de suivre la complexité du binding

Pourquoi conserver le space shuttle style

• Ce contrôleur est le résultat de la fusion, en un seul composant, d’un travail auparavant réparti entre trois contrôleurs
• Lors de la simplification du sous-système PV, il est devenu nécessaire de traiter explicitement toutes les conditions dans le code
• Le résultat peut donc sembler verbeux, avec beaucoup de commentaires et de branches
• Cette verbosité est un moyen de conserver dans le code la connaissance métier et le contexte du comportement de binding
• Lorsqu’on modifie ce fichier, il faut préserver le space shuttle style et, si nécessaire, ajouter des branches et des commentaires selon la même logique

Conception clé : le pointeur bidirectionnel entre PV et PVC

• Au centre de la conception se trouve un pointeur bidirectionnel entre PV et PVC
  • pointeur côté PVC : pvc.Spec.VolumeName
  • pointeur côté PV : pv.Spec.ClaimRef
• Cette bidirectionnalité est difficile à gérer dans un système sans transactions, mais elle est nécessaire pour garantir un fonctionnement correct même en cas de panne
• Si une instance HA défaillante du contrôleur crée une situation de concurrence, plusieurs bindings indiscernables peuvent apparaître, avec un risque de perte de données
• Le contrôleur est fondamentalement conçu pour fonctionner en mode de haute disponibilité active-passive
  • les transitions d’objet sont aussi conçues pour fonctionner en HA active-active
  • toutefois, si deux contrôleurs actifs entrent souvent en conflit, les performances peuvent se dégrader

Méthode de binding et conditions de reprise

• Le contrôleur prend en charge des objets pré-bindés dans les deux sens
  • un PVC qui demande un PV spécifique
  • un PV réservé pour un PVC spécifique
• Le binding se fait en deux étapes
  • d’abord modification de PV.Spec.ClaimRef
  • puis modification de PVC.Spec.VolumeName
• À n’importe quel moment du processus, le PV ou le PVC peut être modifié ou supprimé par un utilisateur ou un autre contrôleur
• Deux contrôleurs ou plus peuvent aussi tenter de binder simultanément des volumes et des claims différents
• Le contrôleur doit pouvoir récupérer de telles situations de conflit

Principaux éléments de la structure du contrôleur

• PersistentVolumeController contient les listers, fonctions de synchronisation des informers, client Kubernetes, enregistreur d’événements, gestionnaire de plugins de volume, etc., nécessaires à la synchronisation PV/PVC
• Les dernières versions connues de PV/PVC sont conservées dans un cache interne
  • volumes persistentVolumeOrderedIndex
  • claims cache.Store
• Ce cache reflète à la fois la version la plus récente stockée sur l’API server et la version reçue via les événements etcd
• Un seul binding peut produire environ quatre événements
  • mise à jour de volume.Spec
  • mise à jour de volume.Status
  • mise à jour de claim.Spec
  • mise à jour de claim.Status
• Sans cache interne, un informer conservant un état obsolète pourrait tenter de corriger à nouveau un binding déjà terminé
• Dans ce cas, une nouvelle écriture vers l’API server peut provoquer un conflit de version avec l’objet déjà stocké

File de travail et contraintes de concurrence

• Le contrôleur dispose de workqueues séparées pour traiter claims et volumes
  • claimQueue
  • volumeQueue
• Chaque file doit avoir exactement un seul thread worker
• En particulier, syncClaim() n’est pas réentrant
• Si deux syncClaim() s’exécutent en même temps, les problèmes suivants peuvent apparaître
  • deux claims différents bindés au même volume
  • un même claim bindé à deux volumes
• Le contrôleur peut récupérer ces situations via les erreurs de version de l’API server et ses propres vérifications, mais une approche multi-worker peut ralentir l’ensemble

syncClaim : point d’entrée de la synchronisation PVC

• syncClaim est la méthode principale appelée lors de la création, mise à jour ou synchronisation périodique d’un claim
• Cette méthode ne distingue pas le type d’événement
• Elle commence par définir sur le PVC la bonne migration annotation et le met à jour sur l’API server si nécessaire
• Elle bifurque ensuite selon la présence de l’annotation AnnBindCompleted
  • sans annotation : syncUnboundClaim
  • avec annotation : syncBoundClaim
• Le traitement réel est séparé entre les méthodes pour claims non bindés et bindés afin d’améliorer la lisibilité

checkVolumeSatisfyClaim : vérification des exigences du PV

• checkVolumeSatisfyClaim vérifie que le PV demandé satisfait les exigences du PVC
• Les conditions vérifiées sont listées explicitement dans le code
  • erreur si le PV a un DeletionTimestamp
  • erreur si la capacité du PV est inférieure à celle demandée par le PVC
  • erreur si storageClassName est différent
  • si le feature gate VolumeAttributesClass est activé, vérification de la correspondance de VolumeAttributesClassName
  • si le feature gate est désactivé mais que le claim ou le volume possède VolumeAttributesClassName, erreur
  • erreur si volumeMode n’est pas compatible
  • erreur si le mode d’accès n’est pas compatible
• Si toutes les conditions sont satisfaites, la fonction renvoie nil

Gestion des événements pour les PVC en delayed binding

• emitEventForUnboundDelayBindingClaim crée un événement informatif pour un claim non bindé en mode delayed binding
• La raison par défaut est WaitForFirstConsumer
• Le message par défaut indique que le binding attend la création du premier consumer
• Si un Pod non encore planifié référence ce PVC, la raison devient WaitForPodScheduled
  • s’il y a plusieurs Pods, le message inclut tous leurs noms
  • le volume scheduling ne prend en compte qu’un seul Pod, mais comme on ne sait pas lequel sera utilisé, ils sont tous mentionnés

syncUnboundClaim : traitement d’un PVC pas encore bindé

• Si claim.Spec.VolumeName est vide, cela signifie que l’utilisateur ne demande pas de PV spécifique
• Dans ce cas, le contrôleur vérifie le mode de delayed binding du claim et cherche le PV le plus adapté avec findBestMatchForClaim
• S’il n’existe pas de PV adéquat, le traitement suit cet ordre
  • si une StorageClass par défaut peut être attribuée, le PVC est mis à jour et la synchronisation s’arrête
  • s’il s’agit d’un delayed binding et que le provisioning n’est pas encore en cours, un événement d’attente est émis
  • si le claim a une StorageClass, tentative de provisioning dynamique avec provisionClaim
  • sinon, enregistrement d’un événement FailedBinding indiquant qu’il n’y a ni PV disponible ni StorageClass
• Si un PV adapté existe, bind est appelé pour binder le PV et le PVC
  • en cas de succès, des métriques sur le provisioning + binding sont enregistrées et le cache de timestamps est nettoyé
  • si une erreur survient pendant l’enregistrement, un syncClaim ultérieur terminera le binding

Traitement d’un PVC qui demande un PV spécifique

• Si claim.Spec.VolumeName n’est pas vide, l’utilisateur a demandé un PV spécifique
• Si le PV demandé n’existe pas dans le cache, le statut du PVC est mis à Pending puis réessayé plus tard
• Si le PV demandé existe et que volume.Spec.ClaimRef est vide, cela signifie que le PV n’est pas encore réclamé
  • les exigences sont vérifiées avec checkVolumeSatisfyClaim
  • si elles ne sont pas satisfaites, un événement VolumeMismatch est enregistré et le PVC reste en Pending
  • si elles sont satisfaites, bind est appelé
• Si le PV demandé est déjà réclamé par ce PVC, bind est appelé pour finaliser le binding
• Si le PV demandé est déjà lié à un autre claim, le traitement est le suivant
  • si le claim n’a pas l’annotation indiquant un binding effectué par le contrôleur, un événement FailedBinding est enregistré et le claim reste en Pending
  • si le binding semble avoir été fait par le contrôleur mais vers un autre claim, une erreur est renvoyée car c’est un état « should never happen »

syncBoundClaim : traitement d’un PVC déjà bindé

• syncBoundClaim traite les PVC qui possèdent l’annotation AnnBindCompleted
• Si un claim déjà bindé a un claim.Spec.VolumeName vide, son état devient ClaimLost
  • le message d’événement indique que le claim bindé a perdu sa référence PV et que les données du volume sont perdues
• Si le PV pointé par le claim n’existe pas, l’état devient également ClaimLost
  • le message indique que le claim bindé a perdu son PersistentVolume et que les données sont perdues
• Si le PV existe mais que volume.Spec.ClaimRef est vide, le volume est considéré comme redevenu non bindé et bind est rappelé
• Si l’UID de PV.ClaimRef correspond à celui du claim, le binding est considéré comme correct et bind est appelé
  • dans la plupart des cas, cet appel ne fait rien
• Si le PV pointe vers un autre claimant, la phase du claim passe à l’état terminal Lost

syncVolume : point d’entrée de la synchronisation PV

• syncVolume est la méthode principale appelée lors de la création, mise à jour ou synchronisation périodique d’un volume
• Elle ne distingue pas non plus le type d’événement
• Elle commence par définir sur le PV la bonne migration annotation et le finalizer approprié, puis met à jour l’API server si nécessaire
• Si volume.Spec.ClaimRef est vide, le volume est considéré comme inutilisé et sa phase passe à Available
• Si ClaimRef existe mais que son UID est vide, le PV est considéré comme réservé pour un PVC spécifique et sa phase passe à Available
  • le PVC n’est pas encore bindé à ce PV, et ce sera traité par la synchronisation PVC

Traitement d’un PV dont le claim est introuvable

• Si un PV est bindé à un claim, le contrôleur cherche le PVC via le namespace/name de ClaimRef
• Si le PVC n’est pas trouvé dans le cache, des vérifications supplémentaires sont effectuées dans certaines conditions
  • nouvelle vérification dans le cache de l’informer
  • nouvelle vérification sur l’API server
• Pour les PV créés par un provisioner PV externe ou un binder PV externe, le PVC peut ne pas encore être synchronisé dans le cache local sous forte charge
• Une double vérification est donc effectuée afin d’éviter de reclaim par erreur un PV
• Si le claim est considéré comme absent, la phase du volume passe à Released puis reclaimVolume est exécuté
  • si la phase existante est Failed, elle n’est pas écrasée
  • si la policy de reclaim est Retain, un log indique que le PV référence un claim inexistant

Quand le lien PV/PVC est désaligné

• Si le claim existe mais que claim.Spec.VolumeName est vide, cela signifie que le PVC n’a pas encore le nom du PV
• Si volumeMode ne correspond pas, un événement VolumeMismatch est enregistré des deux côtés, PV et PVC, et syncClaim est ignoré
• En l’absence de mismatch, le claim est ajouté à claimQueue afin que syncClaim soit appelé rapidement
  • cette méthode accélère le binding des volumes provisionnés
• Si claim.Spec.VolumeName est égal au nom du volume courant, le binding est considéré comme correct et la phase du volume passe à Bound
• Si le claim est bindé à un autre volume, le traitement dépend de la situation
  • pour un volume provisionné dynamiquement avec une policy de reclaim Delete, il est marqué Released puis reclaimVolume est exécuté
  • pour un volume bindé par le contrôleur, le nettoyage se fait via unbindVolume
  • si le pointeur a été créé par l’utilisateur, il est conservé, mais unbindVolume est appelé pour mettre à jour la phase et vider ClaimRef.UID

Mise à jour d’état et émission d’événements

• updateClaimStatus enregistre le status du PVC sur l’API server
  • changement de phase
  • quand il n’y a pas de volume : réinitialisation de AccessModes, Capacity, CurrentVolumeAttributesClassName
  • quand il y a un volume : mise à jour du mode d’accès, de la capacité et du nom courant de la classe d’attributs de volume
• La capacité n’est mise à jour qu’au moment où le claim passe à Bound
  • la différence entre la taille du système de fichiers du PVC et celle du périphérique bloc du PV peut être intentionnelle ; on n’écrase donc pas la capacité d’un claim déjà bindé
• Si le feature gate VolumeAttributesClass est activé, CurrentVolumeAttributesClassName est défini pendant la transition de Pending vers Bound
  • ensuite, cela doit être géré par le resizer ou par un override administrateur ; si le contrôleur continuait à l’écrire, cela pourrait créer une race condition
• updateClaimStatusWithEvent et updateVolumePhaseWithEvent n’émettent des événements que si le status/la phase a réellement changé

Attribution de la StorageClass par défaut

• assignDefaultStorageClass recherche et attribue une StorageClass par défaut lorsqu’un claim n’a pas de classe de stockage
• Les claims qui ont déjà une storage class sont ignorés
• S’il n’existe pas de classe par défaut, la fonction ne met rien à jour et renvoie false
• S’il en existe une, son nom est placé dans claim.Spec.StorageClassName puis enregistré via l’API server

Portée du fichier et limites explicites

• D’après les métadonnées affichées sur GitHub, pv_controller.go fait 2038 lignes, 1864 LOC, 91 KB
• Le contenu fourni ici ne va que du début du fichier jusqu’au début de la fonction bindVolumeToClaim, le reste se poursuivant via un lien vers la vue brute
• Ce résumé se limite donc à la structure du contrôleur, aux commentaires de conception, aux principales branches de synchronisation et à la logique de mise à jour d’état visibles dans le code fourni

[GN⁺]

Avis de Hacker News

• Je ne sais pas s’il est étrange que le code de ce fichier me semble vraiment être du code Go ordinaire. Comme c’est du Go, c’est verbeux, et comme il ne s’appuie pas sur des abstractions profondes, il paraît plus long, mais le code lui-même me semble typique.
  L’abstraction est une arme à double tranchant, donc cette approche me paraît acceptable ; sans l’en-tête, je n’aurais probablement pas réfléchi deux fois au style d’écriture. C’est peut-être une différence liée au fait d’avoir plus d’expérience en logiciel d’entreprise qu’en logiciel système. Pour quelqu’un qui contribue régulièrement à Kubernetes, ces commentaires peuvent sembler inutiles, mais pour du code qui sera lu dans un contexte d’entreprise, par un lecteur lointain dans le futur et sans contexte, j’aurais même probablement ajouté davantage de commentaires à ce niveau de complexité.

  • Avant, ce genre de code paraissait normal, mais depuis environ 10 ans, beaucoup de gens semblent valoriser la concision davantage que l’explicite.
    Surtout dans du code aussi important, je préfère de loin l’explicite. À plusieurs reprises dans ma carrière, à cause de code qui regroupait plusieurs conditions et omettait les commentaires expliquant le contexte métier et le sens, je n’ai pas pu déterminer si le comportement actuel était intentionnel ou accidentel. Cette approche tend à produire non pas du code robuste au changement, mais du code qui empêche le changement, ou du moins qui le rend difficile à modifier par quelqu’un d’autre que l’auteur. Créer une barrière de Chesterton inutile va à l’encontre de la maintenabilité.

  • Ce commentaire a probablement été ajouté après une tentative ratée de simplification du code, comme avertissement aux futurs mainteneurs pour qu’ils y réfléchissent à deux fois avant de tenter la même chose.
    Le commit qui ajoute l’avertissement est "Add note about space-shuttle code style"[1], et le commit juste avant était "Revert controller/volume: simplify sync logic in syncUnboundClaim"[2].

    [1] https://github.com/kubernetes/kubernetes/commit/de4d193d45f6...

    [2] https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca...

  • Je pensais à peu près la même chose, puis j’ai vu les gros blocs if imbriqués et j’ai changé d’avis. Pour cette partie-là, j’aurais clairement créé des branches avec retour anticipé.
    On dirait qu’ils se sont arrêtés à la première étape de « le faire fonctionner, le rendre rapide, le rendre joli », sans passer à « le rendre joli ». J’ai déjà écrit du code laid et très commenté de ce genre pour démêler des interactions d’état délicates, mais en général je le nettoie un peu avant la revue. Peut-être qu’il vaudrait mieux simplement mettre une grande bannière en haut du fichier disant « ne pas essayer de simplifier ce code ». Cela dit, ce n’est clairement pas si mauvais.

  • C’est peut-être étrange, mais tu n’es pas seul. Pour moi aussi, ce code a l’air tout à fait normal. J’ai déjà écrit ce genre de code et de commentaires pour des composants que je considérais comme critiques pour la fiabilité du système.
    Je n’ai jamais adhéré à la mode du « code sans commentaires », et les commentaires que j’avais écrits se sont trop souvent révélés précieux pour mon moi futur quand je revenais dessus des mois ou des années plus tard. J’ai du mal à imaginer reconstituer la logique enfouie dans un composant de cette complexité sans commentaires solides.

  • En particulier, l’affirmation selon laquelle chaque if aurait un commentaire else correspondant ne semble pas être toujours vraie. Une grande partie des if sans correspondant sont de simples tests if (err != nil) { ou d’autres retours anticipés, mais même en les excluant, il semble rester des if sans correspondant.
    Cela dit, d’après mon expérience du logiciel d’entreprise, les commentaires supplémentaires n’étaient pas forcément si nombreux. Les bases de code étaient infestées de commentaires // end if, mais les vrais commentaires explicatifs étaient rares.

• Article sur la qualité du logiciel de la navette spatiale : https://archive.is/HX7n4
  Pour en citer un extrait, ce qui rend ce logiciel remarquable n’est pas la quantité de choses qu’il fait, mais à quel point il fonctionne bien. Il ne plante jamais, n’a pas besoin de redémarrage, n’a pas de bugs et serait aussi proche de la perfection que les humains aient réussi à l’atteindre. Les trois dernières versions comptaient chacune 420 000 lignes et n’avaient qu’une erreur chacune ; les 11 dernières versions au total comptaient 17 erreurs. Un programme commercial de complexité équivalente aurait eu environ 5 000 erreurs, selon l’article.

  • Je me demande ce que signifie exactement « les trois dernières versions faisaient chacune 420 000 lignes et avaient chacune une erreur ». Si chacune des trois versions avait exactement un bug, est-ce que cela ne veut pas dire, formulé bizarrement, que les deux premiers correctifs n’ont pas fonctionné ou ont introduit un nouveau bug ?
  • Il serait intéressant de comparer la méthode de la NASA et celle de SpaceX. SpaceX a aussi mené des missions habitées, donc les exigences semblent assez similaires.
  • 5000 / 17 ≈ 295. Est-il juste de supposer qu’un programme commercial de complexité équivalente a nécessité 295 fois moins de mois-personnes ?
  • Le problème de la méthodologie de développement de la navette spatiale, c’est qu’elle est extrêmement coûteuse et lente, tout en n’étant pas 100 % sans bugs.
    Elle est si coûteuse et si lente que prouver la correction du logiciel avec des assistants de preuve modernes serait beaucoup moins cher, plus rapide et en pratique plus sûr. Des projets comme seL4 et CompCert montrent comment faire.
  • C’est l’un de mes articles préférés. C’est incroyable qu’un texte publié sur Internet en 1996 soit encore accessible.

• Je comprends l’intention derrière // KEEP THE SPACE SHUTTLE FLYING., mais c’est un peu drôle de faire référence dans un commentaire à un système qui n’est plus exploité parce que son bilan de sécurité n’était pas bon.
  Dans une dizaine d’années, les gens se souviendront-ils encore de la navette spatiale en bien ?

  • Les problèmes de sécurité de la navette spatiale étaient pour l’essentiel des problèmes matériels, pas des problèmes logiciels.
    Dans « Appendix F - Personal Observations on Reliability of Shuttle » [0], l’annexe de Richard Feynman au rapport sur l’accident de Challenger en 1986, il écrit ceci :

    En résumé, le système et l’attitude de vérification du logiciel informatique sont de la plus haute qualité. On n’y voit pas le processus caractéristique, observé dans les systèmes de sécurité des Solid Rocket Boosters ou du Space Shuttle Main Engine, par lequel on abaisse progressivement les critères en se trompant soi-même.

    Il a spécifiquement mis en avant la qualité du logiciel avionique comme exemple du fait que même un grand projet gouvernemental complexe comme la navette pouvait être correctement ingénieré, sans être intrinsèquement condamné à la mauvaise qualité et au danger.

0 : https://www.nasa.gov/history/rogersrep/v2appf.htm

• Il a envoyé des personnes et du matériel dans l’espace, puis les a ramenés à la maison, au cours de bien plus de 100 missions réussies. On en garde encore une image positive, et il y a de fortes chances que cela continue. Du point de vue du progrès humain et de l’effet net, c’était une réussite.

• Ce qui a mis fin à la navette, ce n’est pas un mauvais bilan de sécurité, mais le coût et l’anticipation d’une dégradation future de la sécurité.
  Les deux accidents de la navette ont certes tué plus d’astronautes que toute autre catastrophe de la NASA, mais compte tenu de la difficulté réelle de ce qui était accompli, le bilan de sécurité était vraiment remarquable. Le code paraît très bon.

• La situation de la Space Shuttle est plus complexe que de dire simplement que sa sécurité était mauvaise. Si l’on raisonne par mission, son bilan est plutôt meilleur que celui d’autres lanceurs. La navette a connu 2 missions fatales sur 135, tandis que le Soyouz de l’époque soviétique en a connu 2 sur 66, et SpaceShipTwo affiche un bilan effrayant de 1 mission fatale en seulement 12 vols.
  Cela dit, la Space Shuttle avait une capacité d’équipage bien supérieure à ce qui était nécessaire pour la plupart des missions. Contrairement à Apollo ou Soyouz, limités à 3 personnes, elle pouvait en embarquer jusqu’à 8 ; et si l’on considère que la plupart des missions soviétiques/Roscosmos, ESA et CNSA étaient entièrement autonomes et sans équipage, il n’y avait tout simplement pas d’équipage à mettre en danger. Peut-être que cette analogie convient mieux à Kubernetes : un système très ingénieré, puissant et polyvalent, mais qui demande beaucoup d’attention et qui est probablement utilisé un peu plus qu’il ne le faudrait.

• Selon la mesure la plus courante, celle des passagers-milles, la Space Shuttle fait partie des véhicules les plus sûrs jamais construits et ayant volé.
  En tant que personne dont l’enfance s’est précisément déroulée dans les années 1980, je me demande franchement comment on pourrait ne pas en garder un bon souvenir. Est-ce que certains sont trop jeunes pour voir ce programme, toutes ses missions et tous ses accomplissements autrement que rétrospectivement, avec un regard teinté par l’ambiance actuelle centrée sur les prestataires spatiaux privés ?

• Le récit de Richard Hipp sur l’adaptation du code de SQLite aux normes aéronautiques est également assez intéressant : https://corecursive.com/066-sqlite-with-richard-hipp/#testin...

  DO-178B. C’est une norme de qualité pour les produits aéronautiques critiques pour la sécurité... Les tests doivent faire en sorte que chaque opération de branchement du code binaire résultant soit exécutée au moins une fois et prise au moins une fois... Cela a pris un an à raison de 60 heures par semaine... Cela a fait une différence énorme. Pendant les 8 ou 9 années suivantes, il n’y a pratiquement pas eu de bugs.

• Ce passage me fait penser aux vérifications d’exhaustivité dans le code TypeScript. J’essaie toujours de les utiliser.
  https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

  • Le plus récent satisfies never est très adapté à cet usage. C’est aussi pratique si, par préférence, on utilise des chaînes if else.

  • ts-pattern pourrait aussi vous plaire.

    https://github.com/gvergnaud/ts-pattern

• Si l’on ne considère que les cas où chaque if non totalement trivial reçoit un else explicite, je me demande à quel point ce code aurait été simplifié si les auteurs de Kubernetes avaient conçu le tout autour du pattern matching structurel plutôt qu’avec des blocs if/else.
  Plusieurs langages grand public qui prennent en charge le pattern matching structurel disposent d’outils capables de vérifier à la compilation que le matching est exhaustif ; cela seul peut augmenter la densité d’information du code tout en constituant une solution idiomatique.

• Discussion de 2018 : https://news.ycombinator.com/item?id=18772873

• Je n’ai fait que parcourir rapidement le code, mais franchement il ne me paraît pas si mauvais. Il y a des choses que j’aurais faites autrement, mais j’ai vu beaucoup de code bien pire.
  Au moins, ce code suit une règle, tout semble avoir été écrit après réflexion, et on a l’impression qu’il y a une méthode dans ce chaos. Je choisirais sans hésiter ce genre de code plutôt que le fouillis typique que j’ai vu tant de fois, fait de styles mélangés, de code paresseux et de structures illogiques.

• Je me demande pourquoi on invente de nouvelles pratiques de « sécurité » tout en ignorant les bonnes pratiques de génie logiciel documentées.
  Les modules de 2 000 lignes, les méthodes de 200 lignes et les if imbriqués sur 3 ou 4 niveaux sont considérés comme nocifs. Les commentaires qui disent seulement ce que fait le code, et non pourquoi, ne sont pas utiles et risquent de diverger du code réel. On voit aussi des usages inutiles de nil. Sans même entrer dans des problèmes plus profonds comme le couplage ou le principe de responsabilité unique, ces points apparaissent déjà en surface.

  • Si vous pensez que ces choses sont nocives, je vous recommande de lire « John Carmack on Inlined Code ».
    http://number-none.com/blow/john_carmack_on_inlined_code.htm...

    « Le code de contrôle de vol de la fusée d’Armadillo ne faisait que quelques milliers de lignes, alors j’ai pris la fonction principale tic et j’ai commencé à mettre toutes les sous-routines en ligne. Je ne peux pas dire que j’ai trouvé un bug caché susceptible de provoquer un vrai crash, mais j’ai trouvé quelques variables assignées plusieurs fois et quelques flux de contrôle qui semblaient un peu suspects, et le code final était plus petit et plus propre. »

    Si Carmack a trouvé de la valeur dans cette approche, il ne faut probablement pas l’écarter trop vite. Le commentaire de suivi vaut aussi le détour.

    « Dans les années qui ont suivi l’écriture de ce texte, je suis devenu beaucoup plus favorable, même en C/C++, à la programmation fonctionnelle pure dans des limites raisonnables... Quand cela devient difficile à gérer, cherchez un moyen d’extraire un bloc en fonction pure. »

  • Parfois, c’est un cas de « il n’y a pas d’autre solution(TM) ».
    Les limites arbitraires de nombre de lignes ont tendance à créer une fragmentation inutile. Ajoutez les include, les licences, le code de glue et les commentaires, et vous obtenez des spaghettis difficiles à appréhender. Essayez de maintenir des méthodes à 200 lignes dans du code haute performance, et les performances peuvent chuter comme le vol d’Icare.

En lisant les commentaires du code, on voit qu’une énorme quantité de savoir-faire a été intégrée pour simplifier ce code en un module unique, le rendre accessible et, plus important encore, durable. Pour quelqu’un qui ne connaît pas le langage ou la logique, les commentaires qui donnent une vue d’ensemble de ce que fait le code sont très utiles. Six mois plus tard, même son propre code paraît étranger, donc c’est aussi utile pour soi-même.

Les commentaires font partie du code et de la base de code. Si l’on modifie le code autour sans mettre à jour les commentaires en même temps, on introduit un bug de documentation dans le code. Ce n’est pas parce que le compilateur ne les traite pas qu’ils ne sont pas une partie fonctionnelle. Fondamentalement, les commentaires sont de la connaissance et des notes de recherche intégrées au code, et peuvent valoir davantage que le code exécuté lorsqu’il s’agit de maintenir le code écrit.

Les bonnes pratiques ne sont pas des lois ni des règles strictes, mais des lignes directrices. Il faut les appliquer quand elles conviennent à la base de code, et ne pas les suivre aveuglément au point de créer une base de code problématique. Il faut parfois infléchir les règles et en créer soi-même ; si l’on sait ce que l’on fait, c’est tout à fait acceptable.

• J’ai écrit assez longtemps de cette manière « sûre », mais cela m’a fait produire beaucoup plus de bugs qu’une gestion des erreurs en chemin de fer avec retours anticipés, et il m’a aussi fallu bien plus de temps pour les corriger
  Ajouter un else explicite à chaque bloc if fait exploser la complexité, car il faut garder en mémoire le contexte courant. Je pense qu’il serait raisonnable de remplacer cette règle par : « chaque bloc de condition if doit soit effectuer un retour anticipé, soit avoir un bloc else correspondant ». Le motif if (cond) { traitement spécial } est clairement bien plus dangereux que le retour anticipé et rend le raisonnement plus difficile.

• Il n’existe pas un unique ensemble officiel de bonnes pratiques
  La longueur d’une fonction ou le nombre de lignes de code d’un fichier ne sont pas intrinsèquement nuisibles ni bénéfiques. Chaque langage a sa propre vision de la façon d’organiser le code, mais aucune ne peut prétendre être « la meilleure pratique ». Go n’est pas un langage qui privilégie le découpage du code en une multitude de petits fichiers.

• Une méthode de 200 lignes n’est pas intrinsèquement mauvaise. Si le code interne est linéaire et reste au même niveau d’abstraction, cela peut être le meilleur choix
  Créer à la place 40 méthodes de 5 lignes peut être pire. Pour comprendre l’ensemble, il faut sauter d’un endroit à l’autre, et l’on peut aussi se tromper dans l’ordre des appels. Il y a 40! permutations possibles.

• Ce genre de code semble être un candidat idéal pour être déplacé vers un système déclaratif, fondé sur des règles et piloté par des tables
  Cette approche est plus facile à comprendre et à vérifier qu’un code impératif bricolé, saturé de clauses if. Ce type de code désordonné est généralement le signe qu’il manque une abstraction.

  • La philosophie de Go consiste essentiellement à tout écrire de façon assez directe, comme si l’on transposait plus ou moins du code que l’on aurait écrit en C, sans chercher à abstraire quoi que ce soit.