[DOJO-ENSIMAG] Retour - 186e session du Coding Dojo du 10/03/2020

[Rémy Sanlaville]

Bonjour à tous,

Lors de notre séance du 10/03/2020 pour le refactoring à petits pas sur Trivia, Sébastien nous a montré une technique élégante pour refactorer le code afin de déplacer le concept purse dans la classe Game vers la classe Player. Nous avons utilisé cette même technique ensuite pour déplacer le concept

Afin de garder une trace, j'ai noté ci-dessous les différentes étapes à réaliser pour le concept purse (sachant qu'il peut y avoir quelques variantes sur l'ordre qui ne change pas le résultat et le fait qu'on reste toujours au vert).

Merci encore à Sébastien pour nous avoir partager cela.

Rémy

== Refactoring technic: move purse concept from Game to Player class ==
Isolate purse in order to move it to Player

Start with increment purse
1. Isolate increment purse into a method
     Extract ligne 111 purses[currentPlayer]++; into new method addNewPurse()
          Extract Method → All

2. Remove side effect → introduce pure function
Cannot move method addNewPurse() to Player because of side effect. We need to refactor the code in order to have a pure function
    Replace ++ with =
    Extract purses[currentPlayer] + 1 into new method incrementPurse
    Introduce new parameter purse in method incrementPurse for the instruction purses[currentPlayer]

3. Move method incrementPurse to Player class
We need to help IDE in order to move the method to Player class
     Add Player player = currentPlayer(); in method incrementPurse
     Introduce new parameter player in method incrementPurse for the currentPlayer local variable
      Move method incrementPurse to Player class

4. Introduce purses field in Player class
      Introduce Variable int purses = purse + 1;
      Introduce Field purses

5. Clean-up method incrementPurse by removing purse parameter       

        purses = purses + 1;

        Remove the useless purse parameter
            Change signature (PS: Safe Delete does not do the job)

Continue with getting the purse value
1. Isolate get purse into a method
       Extract ligne 114 purses[currentPlayer] into new method purse()
          Extract Method → All

2. Remove side effect → introduce pure function
Cannot move method purse() to Player because of side effect. We need to refactor the code in order to have a pure function
     Add instruction currentPlayer().purse(); in purse() method
     Create method 'purse' in 'Player'

     return purses; in the purse() method of Player class
     replace return purses[currentPlayer]; by return currentPlayer().purse(); in purse() method in Game class

3. Clean-up methods purse() and addNewPurse() in Game class
    Remove purses[currentPlayer] = in addNewPurse() method
    Inline addNewPurse() method
    Inline purse() method

Finally remove purses concept in Game class

    Remove ligne 9 int[] purses = new int[NB_MAX_PLAYERS];

Enjoy !!!

[Mathieu Cans]

Merci Rémy pour ce retour.

Bonne journée et à bientôt.

Mathieu

--
Vous recevez ce message, car vous êtes abonné au groupe Google Groupes "CARA Coding Dojo".
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse cara-dojo+...@googlegroups.com.
Cette discussion peut être lue sur le Web à l'adresse https://groups.google.com/d/msgid/cara-dojo/CAK8JC1SUNbnN8DpndzGf6ZcgV3FVFzt4pSOr4xcw-AwWMu2_Yg%40mail.gmail.com.

[Rémy Sanlaville]

Bonjour à tous,

Avec Bastien nous avons exploré plusieurs pistes pour refaire se refactoring de purses.

Comme une vidéo vaut mieux que beaucoup de texte, nous avons fait une vidéo d'un refactoring que nous avons bien aimé. L'idée est que c'est plus simple de faire la duplication et casser la dépendance du get/set sur purses avant de faire le déplacement vers Player. On vous laisse apprécier le résultat et nous faire des retours.

Bon WE de Pâques !

Rémy et Bastien

PS: A noter quelques améliorations par rapport au premier mail :

- remplacer purse(s) par coin(s) pour le nom des méthodes/attributs. Par exemple :
     addNewPurse→ addCoin

     incrementPurse→ incrementCoin

- Le SafeDelete fonctionne bien, pas besoin de passer par le ChangeSignature.

je ne sais pas pourquoi cela n'a pas été le cas lorsqu'on l'a fait en séance

[Sébastien Canonica]

Ah oui, beaucoup plus simple ! Bravo !

Si je comprend bien, on peut résumer en 4 macro-étapes:

- factoriser les lectures dans un getter et les modifications dans une méthode de mutation

- intercepter le résultat du calcul dans un nouveau champ public de la classe cible (pas besoin de fonction pure, tant que le calcul n'a pas d'effet de bord) -> on a alors une copie à jour de la valeur dans la classe cible

- changer le getter pour retourner le nouveau champ -> on n'a plus besoin du tableau original, on peut nettoyer toutes ces références

- bouger le getter, puis la mutation dans la classe cible

Perso, mais c'est très marginal :

- je ferais l'extract method du coins() tout de suite après celui du addCoin() et le remplacement du ++. Cela permet de bien centraliser toutes les références au tableau dès le départ, d'avoir tout sous les yeux et de pouvoir "oublier" le reste du code.

- au lieu d'extraire la méthode incrementCoin, J'écrirais plutôt directement le nouveau champ en plein milieu

purses[currentPlayer] = currentPlayer().coins = coins() + 1;

La double affectation n'est pas extra dans l'idéal, mais ça matérialise bien le côté "interception". On est pas tellement dans du parallel change à mon avis. Et plus besoin de reinliner ensuite.

[Yannick Chiron]

Merci, le format est cool c'est beaucoup c'est plus clair en vidéo qu'en texte 👍

Il y en a eu du chemin entre la première fois qu'on a fait l'exercice il y a quelques mois et le résultat dans cette vidéo :D

Yannick Chiron

--

Vous recevez ce message, car vous êtes abonné au groupe Google Groupes "CARA Coding Dojo".
Pour vous désabonner de ce groupe et ne plus recevoir d'e-mails le concernant, envoyez un e-mail à l'adresse cara-dojo+...@googlegroups.com.

Cette discussion peut être lue sur le Web à l'adresse https://groups.google.com/d/msgid/cara-dojo/968c32c9-7bef-4114-b141-e5e11e835eed%40googlegroups.com.

[Johan Martinsson]

C'est vraiment chouette et élégant. Ce cas de figure était quelque chose qui était difficile à faire avec les refactorings automatiques mais vous avez trouvé une façon simple d'y arriver

Cette discussion peut être lue sur le Web à l'adresse https://groups.google.com/d/msgid/cara-dojo/CAPwm7LODEVewrTayeq2FbtoBiOAq-sJEiYWhnxe2khxTQLbd%2BA%40mail.gmail.com.