Mon code est moche

C’est ce que j’ai affirmé il y a peu sur Linkedin. Quand je regarde le code que j’ai pondu lorsque j’étais à 42, il était objectivement moche. Il n’était pas totalement incompréhensible, mais revenir dessus était une expérience assez désagréable. C’est un peu comme quand on crée une musique, ou quand on réalise un dessin au tout début. Lorsqu’on regarde quelques années plus tard, on trouve juste ce qu’on a fait horrible et rempli de défauts.

Je n’ai pas pu m’empêcher d’essayer de corriger quelques erreurs et je compte te montrer la différence entre l’avant et l’après.

DISCLAIMER: Ce qu’on ne va pas voir

Il n’y aura pas de code “parfait” ici. J’ai juste décidé de revoir en partie l’écriture d’un parseur pour le projet Wolf3d de 42.

Il n’y aura pas d’écritures de tests ( 😱 )

Il n’y aura pas une architecture très clean ( 😱 😱 )

J’ai voulu me concentrer sur quelques règles importantes de Clean Code, notamment les 3 premiers chapitres. Je vous conseille vivement de le lire si ce sujet vous intéresse.

Explication du projet: Réécriture du parseur de Wolf3d

Qu’est ce que Wolf3d ?

Pour faire simple, il s’agit de réaliser un remake du jeu Wolfenstein en C.

Ce qui nous intéresse aujourd’hui, c’est de parser les fichiers contenant toute l’information sur les maps. Voici un exemple:

• Le 1 signifie qu’il y a un mur, 0 qu’il n’y a rien.
• Les 2 premiers nombres de la dernière ligne correspond à la position sur l’axe X et Y, on aura l’occasion d’y revenir pour un peu plus de précisions.
• Le dernier nombre de la dernière ligne correspond à un angle en degré.

Le but du parseur est de récupérer les trois infos décrites plus hautes. L’ancien code réalisait déjà cela, mais il était très compliqué à comprendre, tu vas vite voir pourquoi.

Let The War Begin

Les bases: le fichier main

Regardons ce fichier main, est-ce que selon toi, c’est clean ?

J’espère que ta réponse est non, sinon tout cet article sera inutile.

Qu’est-ce qui ne va pas sur ce fichier ?

Plusieurs choses:

1. Tout d’abord t_env. A quoi correspond ce type, est-ce qu’il stocke les variables d’environnement du shell ? Ou alors est-ce qu’il stocke toutes les variables nécessaires au bon fonctionnement du programme ?

Il se trouve que c’est la deuxième raison, mais difficile de le savoir à première vu, et c’est surtout à mon sens une solution très sale pour se repérer dans le code.

Regarde la structure de t_env:

Si quelqu’un trouve ça beau/clair/spécifique. Il peut m’envoyer un mail directement martinsiesse@gmail.com.

Le plupart des variables sont indéchiffrables. Quel est la différence entre x/x1/x2/xa/xb/xnew/xold ?

Difficile de s’y retrouver, des meilleurs noms de variables et une meilleur séparation grâce à d’autres structures auraient été bien plus sympa.

2) Également, à quoi correspond w_parsor() ? Pourquoi un w devant parsor, qu’est-ce que cela signifie ? À l’heure actuel, je n’ai toujours pas compris pourquoi il y avait un w.

3) Autre détail, on aurait pu refactoriser cette partie:

Pas besoin d’avoir un if/else en plein milieu du main, le séparer dans une fonction pour décrire précisément ce qu’il se passe aurait été plus judicieux. On peut même remplacer ça par une seule ligne en ternaire.

LA SOLUTION:

Avec ce qu’on vient de décrire, on peut apporter une solution plus clean à ce fichier main:

Tout est plus clair ici.

• La variable stocker va tout simplement stocker toutes les informations qui résulte du parsing. D’où le type ParsorDataStocker.
• openToReadStdinOrMapFile() est la refacto du if/else vu précédemment
• w_parsor() s’est transformé en parsor() et prend uniquement en paramètre le fichier.

Et voilà à quoi ressemble ParsorDataStocker.

C’est beaucoup plus lisible et les noms de variables dans la structure expliquent bien ce que l’on va récupérer dans le fichier.

Tout le long du projet, on fera attention au naming (noms des variables et fonctions), et à la refacto (restructuration logique)

Je ne vais pas te montrer tout ce qui a été réalisé pas à pas, mais j’aimerai te montrer un autre exemple encore plus frappant que le fichier main, qui lui était plutôt sympathique.

Le parsing des lignes et des colonnes ( ⚠️ Risque d’AVC )

Je vais te montrer une des fonctions du parsing qui, quand je l’ai relu pour la première fois, j’ai eu un sacré mal de crâne. Je te montre la fonction, et si tu arrives à comprendre en moins de 2 minutes la logique, c’est que tu n’es pas humain.

Ok, maintenant voici la même fonction, mais avec tout le travail de clean code appliqué dessus. Je peux t’affirmer qu’il se passe la même chose du début à la fin.

4 lignes seulement … Maintenant je pense que tu comprends ce qu’il se passe en moins de 2 minutes, même si c’est peut-être pas 100% clair pour vous à quoi correspond FileReader. Comparé au morceau de code d’avant, une lecture simple suffit à comprendre toute la logique.

Là, tu peux me dire “Non mais attends trop facile, tu as séparé la logique du code de base en plusieurs fonctions, mais si on regarde le détail de l’implémentation, je suis sur qu’on va galérer à comprendre.”

C’est tout l’utilité de bien séparer les fonctions. Je te laisserai l’accès au github du projet, si tu as un bon IDE qui permet de naviguer rapidement entre les fichiers, tu verras que tu comprendras très rapidement ce qu’il se passe et que chacune des fonctions possèdent un rôle bien précis et est parfaitement clair. (je l’espère en tout cas)

Les erreurs de designs de parsor_lcol() sont extrêmement nombreuses. (Fonction trop longue, pas de séparation des différentes logiques, duplication inutile, etc…) et je pense que tu seras d’accord avec moi quand je dis que ce type de code ne peut pas être maintenu sur la durée.

Voyons maintenant un autre réflexe à avoir pour cleaner son code.

La création de constantes

Imagine que je veux rajouter une feature qui prend en compte une troisième dimension ? Ou alors changer la dimension max de ma map. Avec l’ancienne version du parseur ça serait une souffrance.

Dans la nouvelle version, si on veut changer les dimensions min et max possibles pour une map, on a juste à changer les constantes et tout sera modifié dans le code:

Il y a plusieurs avantages à utiliser des constantes:

• On évite la duplication du code
• Les changements de valeurs sont simples
• On sait parfaitement à quoi correspondent ces constantes grâce à un bon naming

Dans l’ancien parseur, ce n’est pas tout de suite clair à quoi correspond 3 et 200:

Dès que tu vois une valeur qui se répète à plusieurs endroits, il y a fort à parié que tu peux la déclarer en tant que constante.

À présent si vous naviguez un peu dans mon code, vous allez peut-être être choqué de ne voir qu’un seul et unique commentaire et tu me diras:

Ton code n’est pas commenté 👎

Voici le seul commentaire du projet:

Il me semblait nécessaire pour préciser le sens de lecture de la map afin de l’adapter à la lecture de la matrice.

Pourquoi ne pas mettre de commentaires à d’autres endroits du code alors qu’on a souvent entendu que c’était important d’avoir un code bien commenté ?

Deux explications:

1. On suppose qu’un code bien commenté = un code bien documenté. Or, il n’y a pas de corrélation entre les deux. La plupart du temps, un code avec des bons noms de fonctions, des bons noms de variables et une séparation claire de la logique entre les fonctions suffisent. Les commentaires peuvent être là pour expliquer un choix d’implémentation ou une subtilité difficilement cernable mais pas pour compenser un code moche.
2. DRY (Don’t repeat yourself). Mettre un commentaire pour expliquer ce que réalise une fonction est la plupart du temps une simple répétition. Exemple:

C’est parfaitement clair en lisant le nom de la fonction, ses paramètres, ainsi que son retour que celle-ci réalise l’addition de a et de b. Le commentaire est inutile, c’est juste une duplication de ce qui est écrit plus bas.

De ce fait, j’ai considéré qu’il était inutile d’ajouter plus de commentaires pour comprendre la logique globale du parseur. Les commentaires devraient être utilisés avec plus de sagesse.

Code Clean = Meilleure vie

J’espère t’avoir convaincu un minimum de l’importance de maintenir un code clean si ce n’était pas déjà le cas.

Je t’invite à regarder plus en détail l’implémentation. Je suis persuadé que ce n’est pas encore parfait et que certaines améliorations sont possibles.

Si ça te titille ou que tu penses que j’ai dit des conneries, n’hésite pas à m’en faire part via Linkedin en message privé ou via mon mail martinsiesse@gmail.com