Livrer du code de meilleure qualité

Bonjour à tous,

Je souhaite aujourd’hui apprendre à livrer en PR dans mon entreprise du code plus robuste / de meilleure qualité. Comme c’est probablement une notion assez vague je vais essayer de vous donner quelques exemples d’erreur que mes collègues relèvent une fois que mon code arrive en PR :

• Des oublis : j’ai oublié de commiter un fichier dans la PR, oublié de retirer une trace de debug, oublié de renommer une fonction ou une variable. Ce dernier cas m’arrive souvent de la manière suivante. J’écris du code avec un bon nommage, je refacto, mais je ne renomme pas mes variables. Du coup mon nommage ne correspond pas à ce qui se passe réellement dans le code.
• Des manques: par exemple parfois je viens implémenter une nouvelle fonctionnalité dans le code, j’écris le code, je la teste, ça fonctionne … mais re-factoriser un peu le code existant dans lequel je m’intègre aurait été plus propre ou alors j’ai allongé une méthode sans me rendre compte qu’elle commençait à devenir trop complexe à maintenir.
• Je propose toujours des tests avec mes PR, mais parfois mes tests ne prennent pas en compte l’ensemble des cas. Ou encore je teste la présence de l’élément A mais pas l’absence de l’élément B

Comme ça fait un bout de temps que je l’ai remarqué j’essaye déjà de toujours bien relire le code avant de le mettre en PR pour éviter les oublis les plus grossiers. Je laisse aussi souvent passer un peu de temps entre le moment ou je termine le code et le moment ou je le relit afin d’avoir un regard plus neuf.

Je sais pas si j’étais super clair sur mon cas . Je suis donc à la recherche de méthodes ou des petites astuces (pourquoi pas des d’outils) que vous utilisez pour progresser sur ce point.

Merci d’avance, Anto

Pour les deux premiers points, un bon IDE peut clairement t’aider. Je ne sais pas dans quel langage tu codes, mais il existe plein d’IDE bien faits qui refactorent intelligemment, et qui gèrent à peu près GIT. Que du bonheur <3

Salut !

Alors pour les oublis, la méthode qui je pense est la plus efficace est de commiter très régulièrement, même du code qui ne fonctionne pas encore, ou au moins le stager. Si tu travailles sur un fichier, que tu vas changer de fichier, tu peux déjà add les modifications que tu as faites, voire les commit si tu peux décrire ce que tu as fait. Des plugins/fonctions intégrées dans ton éditeur seront le saint graal pour toi.

Pour les autres points, j’aurais tendance à dire que c’est de l’expérience. Est-ce que tu fais aussi des code review sur les codes de tes collègues ?

Pour :

Comme ça fait un bout de temps que je l’ai remarqué j’essaye déjà de toujours bien relire le code avant de le mettre en PR pour éviter les oublis les plus grossiers. Je laisse aussi souvent passer un peu de temps entre le moment ou je termine le code et le moment ou je le relit afin d’avoir un regard plus neuf.

J’aurais tendance à dire que tu devrais mettre ton code le plus rapidement possible en PR, quitte à mettre un WIP: devant pour dire que tu travailles dessus. Faire une code review sur 3 modifications c’est toujours plus simple que sur 100 et avoir un avis constructif rapide te permet de corriger plus vite les problèmes.

Je propose toujours des tests avec mes PR, mais parfois mes tests ne prennent pas en compte l’ensemble des cas.

Les tests ne prennent en général jamais en compte l’ensemble des cas. On essaye bien sûr d’utiliser des heuristiques (comme la "couverture", qui consiste à essayer au moins de prendre en compte l’ensemble des chemins dans le graphe de flot de contrôle du programme) pour augmenter la probabilité qu’ils attrapent une erreur s’il y en a une, mais on ne peut jamais en être sûr (c’est Turing qui l’a dit).

Est-ce que du Property Based Testing pourrait alors aider en complément, du moins pour la partie logique métier ?

Ça ne change fondamentalement pas grand chose au problème de base : au lieu d’écrire à la main tout plein de tests, on écrit des propriétés qu’on veut vérifier et on fait confiance à un programme pour qu’il génère automatiquement des tests qu’on espère les plus exhaustifs possible, tout en sachant qu’ils ne peuvent pas l’être.

Parmi les méthodes de test, ça reste évidemment meilleur qu’écrire les tests soi-même à la main : les générateurs sont en général pensés pour pour prévoir les cas un peu vicieux (la liste vide, la division par zéro…), et si c’est impossible d’écrire des tests exhaustifs, c’est déjà difficile de trouver soi-même des tests qui essayent un ensemble des cas suffisamment conséquent (et il ne s’agit pas que de taille) pour qu’on puisse raisonnablement avoir confiance au résultat. C’est aussi plus rapide d’écrire "génère 1000 tests pour vérifier que cette liste a bien les mêmes éléments que celle-là" plutôt qu’écrire ces 1000 tests soi-même. Enfin, ça demande de réfléchir un peu à comment exprimer les propriétés de correction d’une fonction, ce qui n’est pas plus mal.

Après, je ne suis pas spécialement un grand connaisseur des trouzmille méthodes de tests que l’ingénierie logicielle a pu inventer (en partie pour compenser les faiblesses des langages qu’elle a mis en avant par rapport à ceux que j’utilise :-P). Peut-être qu’il y a mieux, peut-être que c’est ce qui marche bien en ce moment. Ce qui est sûr, c’est qu’il faut bien comprendre ce que permettent les tests : ils peuvent parfois montrer qu’un code est faux (il suffit de tomber par hasard sur le bon contre-exemple), jamais qu’il est juste. Ce n’est pas pour ça qu’il s’agit d’une mauvaise pratique ou qu’elle n’a pas d’utilité ¹, il faut simplement être conscient de ses limites (et ne pas courir à tout prix derrière le 100% de couverture en s’imaginant qu’on aura alors testé 100% des comportements possibles).

Merci à tous pour vos réponses

Je code en python avec le framework Django. J’utilise PyCharm comme IDE, avec l’extension Django, je pense que c’est assez complet. Par contre c’est vrai que je ne l’utilise probablement qu’à 10% de ses capacités Je vais me renseigner un peu de ce côté.

On pratique aussi la code review en interne et je trouve que ça permet en effet d’apprendre pas mal de choses, mais plus sur l’aspect technique du language / framework.

D’autres astuces pour les codeurs un peu tète en l’air comme moi ?

Si tu fais déjà attention, perso je te dirais qu’outre peut être mesurer la couverture pour maximiser ce que tu test, le reste n’a pas forcément besoin d’être corrigé spécialement. Tu semble consciencieux. J’ai juste l’impression que ce que tu semble vouloir corriger est justement l’une des raisons pour laquelle on fait du code review. C’est normal de ne pas penser à tout. Avec le temps et l’expérience, si tu continue à faire attention, tu fera moins d’erreur. Mais je ne suis pas sûrs qu’il y ai besoin de faire beaucoup de choses.

Quand j’ai commencé à travailler en entreprise sur des projets versionnés, le simple fait de regarder l’évolution de chaque fichier (via un git diff nomDuFichier par exemple) me permettait de corriger pas mal d’oublie.

Après de manière générale, comme ça a été dit. On en fait de moins en moins avec le temps.
Je pense qu’on en fait plus quand on fait du multi-tache ou du code à gogo.

Regarde également du côté de Pylint, un outil qui permet d’évaluer ton code Python et d’effectuer pas mal de vérifications plus ou moins basiques sur les règles de codage ou de bon sens.

Pour la couverture de code, il existe aussi des solutions type Coverage.

Ensemble, ils permettent déjà de relativement bien gonfler la qualimétrie, à mon sens.

Je suis étonné que personne n’est proposé cette solution toute simple :

Utilise des TODO Lists ! Surtout que Python associé à PyCharm te permettront de les retrouver rapidement. A chaque fois que tu fais un refacto douteux sur tes noms tu fais un TODO juste au dessus en disant "changer le nom". C’est comme écrire sa doc tout de suite. Même si tu la corrige ou la détail plus tard, au moins c’est déjà commencé et acté.

Pour les fichiers oubliés en PR : Je n’ai jamais travaillé en entreprise (à part un petit stage dans un domaine tellement spécifique qu’il n’est pas représentatif) mais vous n’avez pas quelqu’un qui supervise quels sont les fichiers que tu dois retoucher ? Je ne sais pas quelle est la taille du projet et de ton entreprise, mais normalement quand c’est fait particulièrement consciencieusement, quelqu’un (ou toi si personne ne le fait) doit d’abord évaluer quels seront les fichiers à modifier, voire les fonctions, quels seront les fichiers / fonctions à créé(e)(s)… Du coup pour tes PR tu devrais avoir une liste de ce que tu dois envoyer. Mais au delà de ça, en utilisant un gestionnaire de version tu vois ce qui a été modifié et en s’organisant correctement avec le branching tu devrais pas avoir trop de problèmes je pense

Mais rien que le fait que tu te pose la question montre que tu feras partis des bons développeurs - si ce n’est pas déjà le cas - et donc faire partie de 15% meilleurs développeurs.

Je n’ai jamais travaillé en entreprise […] mais vous n’avez pas quelqu’un qui supervise quels sont les fichiers que tu dois retoucher ? Je ne sais pas quelle est la taille du projet et de ton entreprise, mais normalement quand c’est fait particulièrement consciencieusement, quelqu’un (ou toi si personne ne le fait) doit d’abord évaluer quels seront les fichiers à modifier, voire les fonctions, quels seront les fichiers / fonctions à créé(e)(s)…

Heu… non ? Je n’ai jamais vu ça dans mes différentes entreprises et heureusement. On est pas des stagiaires, on doit connaitre notre métier. Si on doit changer un fichier, on le change, si on a pas besoin d’y toucher, on y touche pas. C’est d’autant plus vrai que mis à part les cas triviaux (petite correction de bug où la source est clairement connu), en général tu ne sais pas avant de toucher au code ce que ça affecte globalement, surtout dans un gros projet. Donc établir une liste à l’avance est totalement contre-productif et souvent impossible.

Pour le reste je reste persuadé que ces problèmes restent "normaux" et se corrigent avec le temps et sont une des raisons d’être des review, faut pas trop s’inquiéter de ça.

D’accord !

Eh bien bel erreur de ma part ^^. L’entreprise dans laquelle j’ai fait mon stage fonctionne comme ceci. Avant toute modifications on devait en informer une équipe qui se chargeait de vérifier que c’était nécessaire. Mais peut être est-ce parce qu’il s’agissait du milieu boursier

Probablement… Mais c’est lié à l’organisation du développement. J’avais compris qu’on t’imposais les fichiers à modifier, pas que c’était toi qui proposait (ce qui est différent, c’est toi qui va regarder le soucis pour faire ta proposition).

Dans les boites de développement plus moderne (ou moins contrainte que le boursier), c’est à la review que ça se fait, donc plus a posteriori : les personnes qui font la review vont accepter ou non ces modifs.

Mais bon ça peut revenir au même, sauf que tu dois perdre un temps fou entre leur expliquer pourquoi tu veux modifier un fichier, attendre leur réponse, et prier n’avoir rien oublié lors de ton diagnostique.

Dans le cadre de cette boite pas vraiment. En fait quand on te donne une mission tu dois détailler toute les interractions de ton programme. La machine est tellement complexe (je crois que depuis 1985 il y a du dev) qu’ils doivent être obligés de ne pas toucher à certaines choses. Mais en gros il y a un logiciel qui permet d’indiquer tout ce qui est prévu, ensuite c’est validé. Après tu fais ton dev (tu peux revenir sur ce que tu as demandé plus tôt quand même) puis tu envoie au testing.

Mais je pense que ce monde est habitué à ce genre de chose puisque ceux qui dirigeaient les équipes il y a 10 ans ont codés des millions de lignes avec des cartes perforés