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¹, il était le sujet principal d’un workshop de 2 heures lors de l’excellente conférence NCrafts. Bien après la bataille initiale², à 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.

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

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

public class Diamond {

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

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

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

Faisons le passer aussi simplement que possible.

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

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

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

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

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";
}

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.

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";
}

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.

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();
}

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.

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);
    }
    ...
}

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.

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);
    }
    ...
}

À 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.

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);
     }
     ...
}

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

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;
    }
    ...
}

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

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);
}

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.

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);
}

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.

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;
    }
    ...
}

Et finalement,

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;
}

réflexions

Ma première tentative sur ce kata m’a pris environ deux heures dans un train³.

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⁴.

Le code au complet est disponible sur mon github. Regardez dans les branches :)⁵

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 ?⁶.

Le diamant ‘A’ deviendrait sûrement une constante.

image : By kubotake CC BY 2.0, via Wikimedia Commons. During a solar eclipse, in french, the third contact is also called “Effet diamant” or diamond effect.

Bon, c’était récent quand j’ai commencé à écrire la version originale de cet article. ↩
démarré je pense par cet article Recycling test in TDD de Seb Rose. ↩
sur le chemin de retour de Ncrafts. ↩
et j’en ai fait un paquet. Les décalages +1/-1 sont mon enfer personnel :) ↩
dans la branch 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. ↩
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. ↩

Crafting Labs le blog

Diamond Kata