Skip to content

Conversation

@theoludwig
Copy link
Contributor

Salut Jérémy! 👋 @javascriptdezero

J'ajoute un exercice au coding-dojo dans le niveau moyen, c'est le chiffrement de César expliqué dans l'énoncé de l'exercice.
J'ai fork le projet et je me suis aussi amusé à résoudre chaque exercice existant sur une branche solutions.
Disponible ici : https://github.com/Divlo/coding-dojo

@javascriptdezero
Copy link
Owner

Salut @divlo !

Merci beaucoup pour ta contribution proche de la perfection !!

2 remarques avant que j'intègre ton exercice : peux-tu préciser dans l'énoncé qu'il s'agit de décalages sur la gauche (négatifs) pour la valeur de décalage 3 on passe de D à A, tous tes décalages sont négatifs d'ailleurs dans tes tests.

Proposition (facultatif si tu n'as pas le temps) : on pourrait pousser la difficulté en mettant un décalage positif ou négatif : valeur numérique positive => décalage sur la gauche D => A pour +3 et en valeur numérique négative -3 donne A => D.
Il me semblerait d'ailleurs plus logique de faire l'inverse, pour décalage +3 A => D et -3 D => A mais comme tu as commencé en décalage à gauche pour une valeur positive je comprendrais si tu veux pas changer tous tes tests :-D.

Dernier point : peux-tu supprimer le commentaire au dessus de la déclaration de fonction STP :

/**
 * @param {string} phrase
 * @param {string} decalage
 * @returns {string}
 */

Ça n'apporte pas grand chose au code vu que tout est dans l'énoncé.

Merci beaucoup ! Et encore merci pour ce super taf 👍

@theoludwig
Copy link
Contributor Author

theoludwig commented Dec 23, 2020

Super proposition, je n'avais pas pensé à le faire dans les deux sens de décalages.
Concernant, la JSDOC en commentaires au dessus de la fonction, c'est pour avoir l'autocompletion et le typage de VSCode, comme je suis habité à TypeScript, mais je l'ai enlevé.

J'ai modifié l'énoncé et les tests, faudrait quand même vérifier que les tests sont justes.
Si tu as d'autres changements ou proposition n'hésite pas. 😃

@jeremymouzin
Copy link

Merci d'avoir pris en compte mes retours.

Tu peux vérifier tes tests avec cet outil en ligne : https://cryptii.com/pipes/caesar-cipher

Dernières retouches que je te demanderai :

  1. Supprime la phrase "Vous pouvez voir d'autres exemples dans le fichier des tests : exercice-7.test.js." dans l'énoncé de l'exercice. Remplace-là par d'autres exemples dans la section Exemples de l'énoncé tout simplement ! (Les débutants ne sont pas censés aller farfouiller dans les fichiers *.test.js pour avoir plus d'exemples, il faut leur faciliter la vie)
  2. Pense à mettre un test pour un décalage à 0 ! La sortie doit être égale à l'entrée

Après ça je peux merger ! Merci beaucoup.

@javascriptdezero
Copy link
Owner

Aussi, tu as pas mal d'erreurs dans tes tests unitaires !
Utilise bien le site que je t'ai donné pour vérifier chaque test, merci !

J'ai rédigé le code qui résout l'exercice, je suis dans les starting blocks pour intégrer ton exo, manque plus que tu corriges ces quelques petites choses, merci !

@theoludwig
Copy link
Contributor Author

@javascriptdezero Ah ah, j'avais aussi codé la solution à cet exercice, il y a quelque mois, mais il ne prenait que des nombres positifs avec un décalage à gauche (qui sont maintenant les nombres négatifs, c'est plus logique).
Je vais m'amusé à le recoder une fois merge, en prenant bien en compte nombre négatifs ou positifs.

Pour le moment, j'ai corrigé les tests avec le site que tu m'as conseillé et normalement tout devrait être bon maintenant! 👍

@javascriptdezero
Copy link
Owner

C'est presque bon ! Il reste un test unitaire qui n'est pas bon, dans le fichier exercice-7.test.js ligne 12 la valeur de décalage vaut -12 au niveau du test et 12 au niveau du expect(). On va y arriver !

@theoludwig
Copy link
Contributor Author

C'est presque bon ! Il reste un test unitaire qui n'est pas bon, dans le fichier exercice-7.test.js ligne 12 la valeur de décalage vaut -12 au niveau du test et 12 au niveau du expect(). On va y arriver !

Effectivement, je devrais d'avantage me relire avant de faire un commit. 😅

@javascriptdezero javascriptdezero merged commit 810de60 into javascriptdezero:master Dec 27, 2020
@theoludwig theoludwig deleted the exercice-moyen-7 branch December 27, 2020 10:10
@javascriptdezero
Copy link
Owner

Oui c'est mieux de se relire au moins une fois mais encore merci pour ta contribution et toutes les corrections que tu as apportées suite à mes retours ! Je viens tout juste de merger 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants