Fix de l'issue #54 : Bug de mise à jour de l'état

Saperlu commented 2 years ago

Fix correspondant au bug de l'issue #54

netlify[bot] commented 2 years ago

Deploy Preview for wordle-fr-louan ready!

Name	Link
Latest commit	206ef27052bfa89a6ca93d87f8d79701a499814d
Latest deploy log	https://app.netlify.com/sites/wordle-fr-louan/deploys/6255fd49ac2a7e0008845798
Deploy Preview	https://deploy-preview-55--wordle-fr-louan.netlify.app/
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

richie3366 commented 2 years ago

Bien vu ! Je suppose qu'il était également possible de mettre des Vue.set(...) sur les assignations plus haut, et donc de ne plus avoir à faire le splice d'actualisation. Cela me semblerait plus propre, qu'en penses-tu ? (juste une suggestion/question, ta solution me semble totalement acceptable en l'occurrence)

richie3366 commented 2 years ago

Après avoir relu la doc, le splice est une solution proposée, juste qu'elle me semblait moins élégante, notamment pour faire un retrait-ajout qui avait l'air d'un bricolage plus qu'autre chose. Mais c'est un avis qui est mien et qui ne vaut pas grand chose. Je te laisse me dire ce que tu en penses, et j'irai dans ta direction. Merci en tout cas pour ta contribution très appréciable. cf. https://fr.vuejs.org/v2/guide/reactivity.html#Pour-les-tableaux

Saperlu commented 2 years ago

C'est la première fois que je bidouille du Vue, mais j'ai fait du React dont les paradigmes sont très proches. Ce que tu proposes pourrait être vraiment kif kif avec l'autre alternative dans d'autres situations mais ici, je penche plutôt vers le splice pour plusieurs raisons :

la simplicité : implémenter le set prend plus de lignes (à modifier dans cette PR), pour pas vraiment plus de lisibilité (voir moins ?). En plus, effectuer pas mal d'opérations sur une variable de traitement pour enfin l'allouer à la variable importante, c'est un truc que je fais souvent et ça me parait plus lisible quand tu débarques dans le code : y'a une séparation nette entre l'algo qui calcule le nouveau results et le reste du code (vu que c'est une autre variable qui occupe les lignes).
le déroulement du code : quand on fait les-dites opérations sur currentResult, les résultats ne sont pas calculés dans l'ordre : on peut avoir le résultat de la 5e lettre alors que la 1e lettre est encore à définir. Ce qui fait que le début de l'animation de la 5e lettre va se lancer avant celle de la 1e, et donc que la synchro entre les 5 lettres se fera moins fluidement dans l'animation. Bien sûr, tout cela est très rapide, et donc le problème devient négligeable, mais proportionnellement à la futilité qui occupe notre débat (à savoir, quelle solution, parmi deux tout à fait recevables, est la meilleure pour réparer un bug bénin qui survient quand quelqu'un met deux fois le même mot à un jeu ou la meilleure stratégie est d'essayer des mots différents), je trouve que cela a finalement son importance.

richie3366 commented 2 years ago

D'ac, je merge dès que tu auras retiré les console.log que tu as accidentellement commit en corrigeant le commentaire. ;)

Saperlu commented 2 years ago

Ah mince, bien vu ! ^^

LouanBen / wordle-fr