Avis sur mon code JavaScript : fonctions, boucles et bonnes pratiques

Question

Après avoir fait ton exercice, je me suis lancé (enfin Christophe m'a suggéré !) d'essayer un exercice différent. Me suis donc lancé un petit défi, en essayant de faire plusieurs calculs différents (je pensais pas me compliquer la vie comme ça…). Mais bon j'ai voulu le finir…
Je te donne le lien : https://codepen.io/magikemu/pen/bNemLYe?editors=0010
Si tu as le courage de regarder (le code fonctionne je pense) mais très dur à maintenir (j'en ai discuté avec Christophe ;)
En fait j'aimerais ton avis sur ma compréhension des fonctions, tests, boucles… À savoir, je m'en sors un peu ou c'est plutôt la cata. Sur quoi je dois bosser au vu de ce code ? Quand est-il des fonctions mises au début puis des variables ensuite (je les ajoute au début au fur et à mesure où j'ai besoin d'une variable quand je code).
Bref, le code est-il trop bordélique, ma réflexion bonne ou nulle, bref un petit résumé de ce que ce code donne par rapport à l'apprentissage. Merci en tout cas si tu te penches dessus, je pensais jamais faire autant de lignes !

Louis-Nicolas Leuillet · Accepted Answer

Hello René !
Niveau organisation tu me demandes si c'est OK de mettre les fonctions en haut et les variables ensuite. La convention la plus lisible, c'est l'inverse : d'abord tes déclarations de variables, puis tes fonctions, et enfin ta logique principale ( do...while , switch , etc.).
Après niveau répétion du code comme pour beaucoup de gens qui débutant c'est LE gros point d'amélioration. Par exemple, ce bloc revient quasiment à l'identique dans plusieurs case :
do {
premierChiffre = prompt("Donnez le 1 ier chiffre : ");
} while (premierChiffre < 0 || isNaN(premierChiffre));
premierChiffre = Number(premierChiffre);
Extrais ça dans une fonction réutilisable :
function demanderNombre(message) {
let valeur = "";
do {
valeur = prompt(message);
} while (valeur < 0 || isNaN(valeur));
return Number(valeur);
}
// Utilisation :
premierChiffre = demanderNombre("Donnez le 1er chiffre : ");
Même logique pour tes conversions : tu pourrais regrouper tes taux et labels dans un objet, ce qui simplifierait énormément ton gros switch :
const conversionsArgent = {
1: { taux: 1.18, de: "Euro(s)", vers: "Dollar(s)" },
2: { taux: 0.84, de: "Dollar(s)", vers: "Euro(s)" },
3: { taux: 0.87, de: "Euro(s)", vers: "Livre(s)" },
4: { taux: 1.15, de: "Livre(s)", vers: "Euro(s)" }
};
const conv = conversionsArgent[choixConvertionArgent];
resultat = convertir(premierChiffre, conv.taux);
change1 = conv.de;
change2 = conv.vers;
Concernant le nommage tu t'en sors déjà bien ! Tes noms comme tauxConversionEuroDollar ou premierChiffre sont clairs. Mais renomme change1 / change2 en uniteDepart / uniteArrivee , et affiche en estUneMoyenne avec true / false au lieu de 1 / "" .
Très bonne idée en tout cas d'avoir partagé ton code ! Hésite pas à le faire directement dans un bloc de code sur Believemy à l'avenir.

René Barousse · Answer

Merci pour ta réponse, qui va m'aider, sauf sur un point :
Tu me dis de regrouper dans un objet. Le souci, c'est que je ne sais pas ce que c'est, un objet ; du coup, je ne comprends rien au code :
const conversionsArgent = {
1: { taux: 1.18, de: "Euro(s)", vers: "Dollar(s)" },
2: { taux: 0.84, de: "Dollar(s)", vers: "Euro(s)" },
3: { taux: 0.87, de: "Euro(s)", vers: "Livre(s)" },
4: { taux: 1.15, de: "Livre(s)", vers: "Euro(s)" }
};
const conv = conversionsArgent[choixConvertionArgent];
resultat = convertir(premierChiffre, conv.taux);
change1 = conv.de;
change2 = conv.vers;
Peut-être qu'on voit ça plus tard, mais à ce stade, je suis incapable de comprendre ce que tu fais ;)

René Barousse · Answer

Sur ta réponse : Concernant le nommage, tu t'en sors déjà bien ! Tes noms comme tauxConversionEuroDollar ou premierChiffre sont clairs. Mais renomme change1 / change2 en uniteDepart / uniteArrivee , et affiche en estUneMoyenne avec true / false au lieu de 1 / "" .
Je ne comprends pas le fait de renommer affiche en estUneMoyenne ? En fait, cette valeur est juste là pour l' alert à la fin, avec l'une ou l'autre réponse (c'est pour ça que j'ai mis affiche : ça concerne l'affichage). estUneMoyenne concernerait plutôt un rapport à des chiffres ? Pour true et false , OK, je vois le principe.

Louis-Nicolas Leuillet · Answer

Le souci avec affiche , c'est que c'est très vague. Quand quelqu'un lit ton code sans contexte, il tombe sur une variable affiche et il se demande : "affiche quoi ?" Est-ce que c'est un booléen qui dit si on affiche quelque chose ? Est-ce que c'est le texte à afficher ? On ne sait pas.
Ma suggestion estUneMoyenne venait du fait que, si j'ai bien compris ton code, cette variable sert à déterminer si le résultat final doit être présenté comme une moyenne ou comme un résultat classique. Du coup, le nom décrit la décision que porte la variable, pas ce qu'on en fait après.
C'est une convention assez répandue : quand une variable est un booléen, on la nomme sous forme de question. estUneMoyenne → "est-ce que c'est une moyenne ? oui ou non". estUneConversion , estTermine , etc. Ça rend le code super lisible quand tu arrives dans ton if :
if (estUneMoyenne) {
// on affiche le résultat façon moyenne
} else {
// on affiche le résultat classique
}
Tu vois, ça se lit presque comme du français. Et pour les objets dont je parlais dans mon message précédent, oublie ça pour le moment, tu verras ça plus tard dans la formation et ça deviendra limpide.

René Barousse · Answer

En fait, la variable "affiche" me sert à afficher uniquement si tu choisis l’option 4 au départ (calculer une moyenne).
alert("La moyenne de vos " + choixCalculMoyenne + " nombres est de : " + resultat);
Je n’ai pas trouvé d’autre solution pour afficher cette alert qui n’est pas la même que si tu fais le choix 1, 2 ou 3. Il me permet juste d’afficher un message différent à la fin, qui normalement est :
alert("Le resultat de votre calcul est : " + premierChiffre + " " + change1 + " = " + resultat + " " + change2);
Et pour ce qui est du truc objet que tu as mis ?

Louis-Nicolas Leuillet · Answer

Oui, j'ai bien compris le principe : ta variable affiche sert de "flag" pour savoir quel alert afficher à la fin.
Ce que je te suggérais, c'est simplement de renommer cette variable pour qu'elle soit plus parlante. Même si elle sert à conditionner l'affichage, le nom affiche ne dit pas pourquoi on affiche différemment. Avec estUneMoyenne et des valeurs true / false , ta condition finale devient :
if (estUneMoyenne) {
alert("La moyenne de vos " + choixCalculMoyenne + " nombres est de : " + resultat);
} else {
alert("Le resultat de votre calcul est : " + premierChiffre + " " + change1 + " = " + resultat + " " + change2);
}
Ça se lit naturellement : "si c'est une moyenne, on affiche le message moyenne, sinon on affiche le message classique". Le comportement est strictement le même que ce que tu avais, c'est juste plus clair pour quelqu'un qui lirait ton code (ou pour toi-même dans 3 mois). Mais bon, on est vraiment sur du détail de nommage, l'important c'est que la logique est bonne et que tu as compris le principe du flag conditionnel.
Et pour les objets, on oublie pour le moment ! Tu les verras plus loin dans la formation, et à ce moment-là tu pourras revenir sur ce code et le simplifier. Ce sera un super exercice d'ailleurs.

René Barousse · Answer

ok

Avis sur mon code JavaScript : fonctions, boucles et bonnes pratiques

Formation JavaScript (legacy)