2018-01-29-diamond-kata.md 15.8 KB
Newer Older
Antoine's avatar
Antoine committed
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455
---
layout: post
title: "Diamond Kata"
date: 2018-01-29 15:44:00 +0100
lang: fr
ref: diamond_kata
categories:
 - "kata"
 - "tdd"
 - "code"
note: "> Il s'agit d'une traduction d'[un article](/2015/08/18/diamond-kata/) initialement écrit en anglais en 2015"
---
[![By kubotake [CC BY 2.0 (http://creativecommons.org/licenses/by/2.0)], via Wikimedia Commons](https://upload.wikimedia.org/wikipedia/commons/thumb/5/55/Kubotake_-_Diamond_ring_on_22_Jul._2009_%28by%29.jpg/320px-Kubotake_-_Diamond_ring_on_22_Jul._2009_%28by%29.jpg)](https://commons.wikimedia.org/wiki/File:Kubotake_-_Diamond_ring_on_22_Jul._2009_%28by%29.jpg){: .left-image}

## le problème : le kata diamond

J'utilise la description de Seb Rose, simple et efficace.

>Étant donnée une lettre, afficher une diamant commençant par 'A' avec la lettre donnée au point le plus large.

> Par exemple : print-diamond 'C' affiche :

      A
     B B
    C   C
     B B
      A


J'ai entendu beaucoup de chose au sujet de ce kata. Récemment[^1], il était le sujet principal d'un workshop de 2 heures lors de l'excellente conférence [NCrafts](http://ncrafts.io).
Bien après la bataille initiale[^2], à mon tour de m'y coller

## ma solution

### l'idée

À chaque fois qu'on me dit que quelque chose est difficile voir impossible, j'ignore cette information et tente le coup comme je le ferais si on ne m'avait rien dit.
Donc j'applique l'approche TDD habituelle : rouge, vert, refactor. Et c'est tout.

Écrire un test, le faire passer, nettoyer le code. Une étape à la fois.

### le premier test

Commençons donc par le premier test.

{% highlight java %}
public class DiamondTest {
    @Test public void 
    should_draw_diamond_A() {
        assertEquals("A", Diamond.create('A'));
    }
}
{% endhighlight %}

Le test est rouge, il est temps de le faire passer au vert.

{% highlight java %}

public class Diamond {

    public static String create(Character c) {
        return "A";
    }   
}
{% endhighlight %}

Yep, c'est une implémentation totalement stupide. Mais ça marche :)

Et relativement propre en lui même. Ajoutons donc un autre test.

### a second test

{% highlight java %}
    @Test public void 
    should_draw_diamond_B() {
        assertEquals(" A \nB B\n A ", Diamond.create('B'));
    }
{% endhighlight %}

Faisons le passer aussi simplement que possible.

{% highlight java %}
    public static String create(Character c) {
        if (c.equals('B')) {
            return " A \n"
                 + "B B\n"
                 + " A ";
        }
        
        return "A";
    }       
{% endhighlight %}

Et c'est vert. C'est le moment de refactorer.

Ok, il y a deux fois la même ligne " A " dans le diamant 'B', mais je n'arrive pas à lui donner un sens. Je n'ai probablement pas assez de données.

Donc je garde les choses comme ça pour l'instant et ajoute un troisième test.

# et le troisième

{% highlight java %}

@Test public void 
should_draw_diamond_C() {
    assertEquals("  A  \n"
               + " B B \n"
               + "C   C\n"
               + " B B \n"
               + "  A  ", Diamond.create('C'));
}

{% endhighlight %}

Comme j'essaie d'être cohérent, ma façon de le faire passer vert ne sera pas différente des précédentes :)

{% highlight java %}
public static String create(Character c) {
    if (c.equals('C')) {
        return "  A  \n"
             + " B B \n"
             + "C   C\n"
             + " B B \n"
             + "  A  ";
    }

    if (c.equals('B')) {
        return " A \n"
             + "B B\n"
             + " A ";
    }
    
    return "A";
}
{% endhighlight %}

Yep, encore une fois, c'est une implémentation simpliste.
À ce moment, je ne me soucis pas d'être intelligent, ni de faire les choses proprement.
À ce moment, je veux juste être sûr que le test que je viens d'écrire est bien celui que j'avais l'intention d'écrire.

En le faisant passer au vert avec une solution simple, il y a peu de risques que j'introduise une erreur dans l'implémentation. Ainsi quand je vois le test passer de rouge à vert, je peux avoir confiance dans mon test.

C'est ma façon de tester mes tests.

Bon, maintenant, j'ai peut-être assez de matière pour vois si je peux faire apparaître des choses intéressantes.

### le temps de commencer à refactorer.

Il doit y avoir une logique dans la distribution des lettres et des espaces.
Comme je ne la comprends pas encore, je vais artificiellement les séparer. Oui, je suis sur le point de créer volontairement et artificiellement de la duplication.


{% highlight java %}
public static String create(Character c) {
    if (c.equals('C')) {
        return "  " +        "A"  +       "  " + "\n"
             +  " " + "B" +  " "  + "B" + " "  + "\n"
             +   "" + "C" + "   " + "C" + ""   + "\n"
             +  " " + "B" +  " "  + "B" + " "  + "\n"
             + "  " +        "A"  +       "  ";
    }
    
    if (c.equals('B')) {
        return " " + "A" + " " + "\n"
             + "B" + " " + "B" + "\n"
             + " " + "A" + " ";
    }
    
    return "A";
}
{% endhighlight %}

Maintenant, je vois que la première et la dernière ligne suivent le motif `z spaces - A - z spaces`.
Si un diamant 'A' a une taille de 1, 'B' de 2, 'C' de 3 et ainsi de suite, alors `z = (size - 1)`.

Parfait. Ajoutons cette logique dans le code.

{% highlight java %}
public static String create(Character c) {
    int size = c - 'A' + 1;

    if (c.equals('C')) {
        return diamondTip(size) + "\n"
             +  " " + "B" +  " "  + "B" + " "  + "\n"
             +   "" + "C" + "   " + "C" + ""   + "\n"
             +  " " + "B" +  " "  + "B" + " "  + "\n"
             + diamondTip(size);
    }
    
    if (c.equals('B')) {
        return diamondTip(size) + "\n"
             + "B" + " " + "B" + "\n"
             + diamondTip(size);
    }
    
    return "A";
}

private static String diamondTip(int size) {
    return manySpaces(size -1) + "A" + manySpaces(size - 1);
}

private static String manySpaces(int nbSpaces) {
    StringBuilder builder = new StringBuilder();
    
    for (int i = 0; i < nbSpaces; i++) {
        builder.append(" ");
    }
    return builder.toString();
}

{% endhighlight %}

Je remarque aussi que les autres lignes suivent un autre motif : `x spaces - a char - y spaces - same char - x spaces`.
Faisons le apparaître dans le code.

{% highlight java %}
public static String create(Character c) {
    ...
    if (c.equals('C')) {
        return diamondTip(size) + "\n"
             +  manySpaces(1) + "B" + manySpaces(1) + "B" + manySpaces(1) + "\n"
             +  manySpaces(0) + "C" + manySpaces(3) + "C" + manySpaces(0) + "\n"
             +  manySpaces(1) + "B" + manySpaces(1) + "B" + manySpaces(1) + "\n"
             + diamondTip(size);
    }

    if (c.equals('B')) {
        return diamondTip(size) + "\n"
             + "B" + manySpaces(1) + "B" + "\n"
             + diamondTip(size);
    }
    ...
}
{% endhighlight %}

Il y a indéniablement un truc qui lie tout ça. Mais je ne le vois pas encore.


### a fourth test

Ajoutons le diamant 'D'. Je ne vous montre pas le test, c'est le même que les précédent mais avec un diamant 'D'.
Faisons le passer en ajoutant cela à la solution actuelle.

{% highlight java %}
public static String create(Character c) {
    ...
    if (c.equals('D')) {
        return diamondTip(size) + "\n"
                + manySpaces(2) + "B" + manySpaces(1) + "B" + manySpaces(2) + "\n"
                + manySpaces(1) + "C" + manySpaces(3) + "C" + manySpaces(1) + "\n"
                + manySpaces(0) + "D" + manySpaces(5) + "D" + manySpaces(0) + "\n"
                + manySpaces(1) + "C" + manySpaces(3) + "C" + manySpaces(1) + "\n"
                + manySpaces(2) + "B" + manySpaces(1) + "B" + manySpaces(2) + "\n"
                + diamondTip(size);
    }
    ...
}
{% endhighlight %}

À partir de maintenant, je ne montrerais que le refactoring qui a lieu sur le diamant 'D'.

Tous ces entiers doivent avoir une sorte de relation. Essayons de les réarranger.

Introduisons `width` : c'est la largeur d'un diamant.
Après avoir dessiné quelques diamant sur une feuille de papier, j'en conclus que la largeur est `size * 2 - 1`.


{% highlight java %}
public static String create(Character c) {
    int size = c - 'A' + 1;
    int width = size * 2 - 1;
    
    if (c.equals('D')) {
        return diamondTip(size) + "\n"
            + manySpaces(2) + "B" + manySpaces(width - 2*3) + "B" + manySpaces(2) + "\n"
            + manySpaces(1) + "C" + manySpaces(width - 2*2) + "C" + manySpaces(1) + "\n"
            + manySpaces(0) + "D" + manySpaces(width - 2*1) + "D" + manySpaces(0) + "\n"
            + manySpaces(1) + "C" + manySpaces(width - 2*2) + "C" + manySpaces(1) + "\n"
            + manySpaces(2) + "B" + manySpaces(width - 2*3) + "B" + manySpaces(2) + "\n"
            + diamondTip(size);
     }
     ...
}
{% endhighlight %}

Bien, je crois que je l'ai. Faisons apparaître une notion d'étage (`floor`)

{% highlight java %}
public static String create(Character c) {
    ...
    if (c.equals('D')) {
        String diamond = diamondTip(size) + "\n";

        int floor = 3;
        diamond += manySpaces(floor - 1) + "B" + manySpaces(width - 2*floor) + "B" + manySpaces(floor - 1) + "\n";
        floor = 2;
        diamond += manySpaces(floor - 1) + "C" + manySpaces(width - 2*floor) + "C" + manySpaces(floor - 1) + "\n";
        floor = 1;
        diamond += manySpaces(floor - 1) + "D" + manySpaces(width - 2*floor) + "D" + manySpaces(floor - 1) + "\n";
        floor = 2;
        diamond += manySpaces(floor - 1) + "C" + manySpaces(width - 2*floor) + "C" + manySpaces(floor - 1) + "\n";
        floor = 3;
        diamond += manySpaces(floor - 1) + "B" + manySpaces(width - 2*floor) + "B" + manySpaces(floor - 1) + "\n";

        diamond += diamondTip(size);
                
        return diamond;
    }
    ...
}
{% endhighlight %}

Impeccable, toutes les lignes se ressemble maintenant. On peut lui donner un nom.

{% highlight java %}
public static String create(Character c) {
    ...
    if (c.equals('D')) {
        String diamond = diamondTip(size) + "\n";
        diamond += diamondWall(size, 3, "B") + "\n";
        diamond += diamondWall(size, 2, "C") + "\n";
        diamond += diamondWall(size, 1, "D") + "\n";
        diamond += diamondWall(size, 2, "C") + "\n";
        diamond += diamondWall(size, 3, "B") + "\n";
        diamond += diamondTip(size);
                                     
        return diamond;
    }

    ...
}

private static String diamondWall(int size, int floor, String wall) {
    int width = size * 2 - 1;
    return manySpaces(floor - 1) + wall + manySpaces(width - 2*floor) + wall + manySpaces(floor - 1);
}
{% endhighlight %}

C'est mieux, mais ça n'est pas encore fini.
Il y a un motif dans ces appels. Voyons voir ce qui arrive si on renumérote les étages de façon à ce qu'ils soit en séquence de `+size` à `-size`.

{% highlight java %}
public static String create(Character c) {
    ...
    if (c.equals('D')) {
        String diamond = diamondTip(size) + "\n";
        diamond += diamondWall(size, 2, "B") + "\n";
        diamond += diamondWall(size, 1, "C") + "\n";
        diamond += diamondWall(size, 0, "D") + "\n";
        diamond += diamondWall(size, -1, "C") + "\n";
        diamond += diamondWall(size, -2, "B") + "\n";
        diamond += diamondTip(size);
                             
        return diamond;
    }
    ...
}

private static String diamondWall(int size, int floor, String wall) {
    int width = size * 2 - 1;
    int absoluteFloor = Math.abs(floor);

    return manySpaces(absoluteFloor) + wall + manySpaces(width - 2*(absoluteFloor + 1)) + wall + manySpaces(absoluteFloor);
}
{% endhighlight %}

On dirait bien qu'il y a une boucle de cachée là dedans. Ce qui nous empêche de l'introduire est le caractère en paramètre de l'appel à `diamondWall()`.
Mais on peut facilement s'en débarrasser puisque le caractère utilisé à un étage peut être calculé à partir de la taille du diamant et l'étage actuel.

{% highlight java %}
private static String diamondWall(int size, int floor) {
    int width = size * 2 - 1;
    int absoluteFloor = Math.abs(floor);
        
    Character wall = Character.toChars('A' + size - absoluteFloor -1)[0]; 

    return manySpaces(absoluteFloor) + wall + manySpaces(width - 2*(absoluteFloor + 1)) + wall + manySpaces(absoluteFloor);
}

public static String create(Character c) {
    ...

    if (c.equals('D')) {
        String diamond = diamondTip(size) + "\n";
        diamond += diamondWall(size, 2)  + "\n";
        diamond += diamondWall(size, 1)  + "\n";
        diamond += diamondWall(size, 0)  + "\n";
        diamond += diamondWall(size, -1) + "\n";
        diamond += diamondWall(size, -2) + "\n";
        diamond += diamondTip(size);
                             
        return diamond;
    }
    ...
}
{% endhighlight %}

Et finalement,

{% highlight java %}
public static String create(Character c) {
    int size = sizeOfDiamond(c);
    
    if(size == 1) return "A";
    
    String diamond = diamondTip(size) + "\n";
    
    for (int floor = size - 2; floor >= -(size - 2) ; floor --) {
        diamond += diamondWall(size, floor)  + "\n";
    }
    
    diamond += diamondTip(size);
    
    return diamond;
}
{% endhighlight %}

## réflexions

Ma première [tentative sur ce kata](https://github.com/avernois/kata-diamond_java/tree/2015.05.23-first_attempt) m'a pris environ deux heures dans un train[^6].

Comme à mon habitude, j'ai fait le refactoring avec de toutes petites étapes de façon à conserver mes tests verts tout le temps
J'ai essayé de laisser l'algorithme émerger du refactoring et non l'inverse.

Bien sur, il n'émerge pas de lui même. Avec ce type de problème, j'ai dans l'idée qu'il va y avoir une forme de boucle dans la solution. 
Mes refactorings ont donc pour but de faire apparaître cette boucle en introduisant volontairement de la duplication pour que chacune des lignes se ressemblent et pouvoir ensuite unifier les indices.

En ne faisant qu'un changement à la fois, les tests sont un outils très efficaces pour m'aider à trouver les erreurs que je fais[^7].

Le code au complet est disponible sur [mon github](https://github.com/avernois/kata-diamond_java). Regardez dans les branches :)[^8]

## ensuite

Une fois rendu là, si c'était du code de production, je donnerais probablement un nom à `size -2` et `-(size - 2)`. Peut-être 'grenier' et 'cave' pour rester dans la métaphore du bâtiment `size` devrait alors probablement devenir `height` (hauteur).

Je n'aime pas trop l'utilisation de `Character` un peu partout alors que cela représente en fait un type de diamant. J'introduirais alors une classe `DiamondKind` pour contentir ce `Character` ainsi que `size` et `width`. Peut-être d'autres opérations sur `Character`.

Aussi une classe `Floor` ?[^5].

Le diamant 'A' deviendrait sûrement une constante.

------
image : By kubotake [CC BY 2.0](http://creativecommons.org/licenses/by/2.0), via Wikimedia Commons. During a solar eclipse, in french, the third contact is also called "Effet diamant" or diamond effect.



[^1]: Bon, c'était récent quand j'ai commencé à écrire la version originale de cet article.
[^2]: démarré je pense par  cet article [Recycling test in TDD](http://claysnow.co.uk/recycling-tests-in-tdd/) de Seb Rose.
[^3]: ou `heigth` hauteur
[^4]: comme `charForFloor`
[^5]: mais actuellement l'utilisation de `floor` est un détail d'implémentation. Je ne suis pas sur que ce soit une bonne idée de lui donner sa propre classe qui deviendrait publique.
[^6]: sur le chemin de retour de [Ncrafts](http://ncrafts.io).
[^7]: et j'en ai fait un paquet. Les décalages +1/-1 sont mon enfer personnel :)
[^8]: dans la [branch 2015.05.23-step_by_step](https://github.com/avernois/kata-diamond_java/tree/2015.05.23-step_by_step), j'ai fait un commit pour chaque étape avec des explications sur l'objectif de cette étape. Pour le principal, c'est ce que vous retrouvez dans ce billet.