Peut-on faire du TDD sur du code existant ?

Notre quotidien de développeur consiste très souvent à modifier du code existant. Certes, nous avons parfois la chance de développer de nouveaux modules tout frais, tout neufs et le Test Driven Development est à son avantage.

Mais comment peut-on mettre en pratique le TDD sur du code déjà écrit, parfois mal pensé et non testé. Cet article va partir d’un exemple concret, une classe comme on pourrait en trouver sur tous les projets. Le but est de commencer une démarche TDD et de pousser au maximum son efficacité (mise en évidence d’un bug potentiel, ajout d’une nouvelle fonctionnalité, refactoring du code et refactoring des tests). Au final, la classe sera beaucoup plus apte à subir le changement et surtout, elle sera testée et couvrira tous les cas d’utilisation.

Au commencement était le code existant

L’exemple est tiré du site The Daily WTF. C’est en regardant la photo ci-dessous que je me suis dit que le code serait un bon candidat au refactoring. Il n’y a pas de frameworks, de règles de gestion complexes et c’est pourtant ce genre de code « simple » qui peut nous ralentir au quotidien.

Vous l’aurez compris, il s’agit d’associer des mots que nous appellerons flag à des valeurs booléennes : par exemple ON correspond à true, OFF à false. De plus, les flags peuvent être orthographiés différemment : ON, On, on doivent tous correspondre à true.

Voici l’adaptation Java du code legacy ci-dessus. J’ai introduit la méthode publique asBoolean(), utilisée par le code appelant, renvoyant la valeur booléenne d’un flag. Si aucun flag ne correspond, la méthode renvoie null.

public class StringSettingBool {

    private Map<String, Boolean> boolFlag = new HashMap<String, Boolean>() {
        {
            put("NO", false);
            put("No", false);
            put("no", false);
            put("YES", true);
            put("Yes", true);
            put("yes", true);
            put("OFF", false);
            put("Off", false);
            put("off", false);
            put("ON", true);
            put("On", true);
            put("on", true);
        }
    };

    public Boolean asBoolean(String flag) {
        if (boolFlag.containsKey(flag)) {
            return boolFlag.get(flag);
        }
        return null;
    }
}

Le Cycle TDD

Pour rappel, le TDD consiste en 3 étapes :

  1. Écrire un test qui échoue
  2. Écrire le code suffisant pour passer le test
  3. Refactorer le code

Couverture du legacy

Dans notre cas, nous devons d’abord écrire une suite de tests couvrant le code déjà écrit avant de pouvoir le modifier (correction d’anomalie, refactoring, ajout d’une évolution).

Voici les tests que j’ai écrit :

public class StringSettingBoolTest {

    StringSettingBool stringSettingBool = new StringSettingBool();

    @Test
    public void should_be_false_for_no_flag() {
        for (String noFlag : { "NO", "No", "no" })
            assertFalse(stringSettingBool.asBoolean(noFlag));
    }

    @Test
    public void should_be_false_for_off_flag() {
        for (String offFlag : { "OFF", "Off", "off" })
            assertFalse(stringSettingBool.asBoolean(offFlag));
    }

    @Test
    public void should_be_true_for_yes_flag() {
        for (String yesFlag : { "YES", "Yes", "yes" })
            assertTrue(stringSettingBool.asBoolean(yesFlag));
    }

    @Test
    public void should_be_true_for_on_flag() {
        for (String onFlag : { "ON", "On", "on" })
            assertTrue(stringSettingBool.asBoolean(onFlag));
    }

    @Test
    public void should_return_null_when_flag_is_null() {
        assertNull(stringSettingBool.asBoolean(null));
    }

    @Test
    public void should_return_null_when_flag_is_unknown() {
        assertNull(stringSettingBool.asBoolean("garbage"));
    }

    @Test
    public void should_return_null_when_flag_is_empty() {
        assertNull(stringSettingBool.asBoolean(""));
    }

}

Comme vous pouvez le remarquer, je vérifie simplement les deux règles précédemment citées :

  1. Les flags doivent correspondre aux valeurs booléennes attendues (yes -> true, no -> false, …)
  2. La valeur null doit être retournée en cas d’utilisation d’un flag invalide (valeur nulle ou inconnue ou chaîne vide).

Démarrage du cycle

Pour les besoins de l’article, nous imaginerons qu’un bug a été détecté par l’équipe QA. L’application n’a pas le comportement attendu si la valeur nO est passé en paramètre d’un formulaire.

TDD étape 1 – Écrire le test qui échoue

Si le flag NO est orthographié nO, la méthode asBoolean() ne renverra pas false mais null. Comme première correction nous pouvons rendre la méthode asBoolean() insensible à la casse.

Nous faisons du TDD, nous écrirons donc un test qui doit mettre en évidence le bug.

@Test
public void should_be_false_for_no_flag_ignore_case() {
    assertFalse(stringSettingBool.asBoolean("nO"));
}

Le test échoue, c’est l’occasion de toucher au code !

TDD étape 2 – Faire passer le test

Rien de bien compliqué, nous allons convertir en minuscule le flag reçu en paramètre.

public Boolean asBoolean(String flag) {
        if (flag == null) {
            return null;
        }
        flag = flag.toLowerCase();
        if (boolFlag.containsKey(flag)) {
            return boolFlag.get(flag);
        }
        return null;
    }

Le test est vert, profitons-en pour réduire la quantité de code !

TDD étape 3 – Refactorer

Depuis que nous utilisons les flags en minuscule, nous pouvons supprimer toutes les insertions superflues. Le code lié à l’initialisation de la Map est désormais plus concis.

private Map<String, Boolean> boolFlag = new HashMap<String, Boolean>() {
    {
        put("no", false);
        put("yes", true);
        put("off", false);
        put("on", true);
    }
};

Nouvelle fonctionnalité

Il devient dorénavant plus simple d’ajouter un flag comme par exemple start.

TDD étape 1 – Écrire le test qui échoue

On commence par écrire les tests qui couvrent le flag start

@Test
public void should_be_true_for_start_flag() {
    String[] startFlags = { "START", "start", "Start", "STart", "STArt" };
    for (String startFlag : startFlags) {
        assertTrue(stringSettingBool.asBoolean(startFlag));
    }
}

TDD étape 2 – Faire passer le test

Une seule ligne est nécessaire (contre trois dans la version originale) :

put("start", true);

TDD étape 3 – Refactorer, les tests cette fois-ci !

La batterie de tests ne comporte pas toutes les possibilités d’écriture des flags (ex: STARt, StArT, STARt, …). Nous pourrions profiter d’une fonctionnalité de JUnit permettant de paramétrer l’exécution des tests avec un jeu de données préalablement généré. Ce que nous voulons, c’est qu’un test prenne en paramètre un flag et la valeur booléenne attendue en résultat de l’appel à la méthode asBoolean(). Le jeu de données serait constitué de toutes les combinaisons de casses possibles d’un flag.

@RunWith(Parameterized.class)
public class StringSettingBoolTest {

    StringSettingBoold stringSettingBool = new StringSettingBool();

    private String flag;
    private boolean expectedBooleanValue;

    public StringSettingBoolTest(String flag, boolean expectedBooleanValue) {
        this.flag = flag;
        this.expectedBooleanValue = expectedBooleanValue;
    }

    @Test
    public void should_return_boolean_value_for_valid_flag() {
        assertEquals(flag + " should be " + expectedBooleanValue, expectedBooleanValue,
                stringSettingBool.asBoolean(flag));
    }

    @Parameters
    public static Collection<Object[]> createDataSet() {
        Collection<Object[]> data = new ArrayList<Object[]>();
        addItem(data, false, "no", "off");
        addItem(data, true, "yes", "on", "start");
        return data;
    }

    private final static StringUtil stringUtil = new StringUtil();

    private static void addItem(Collection<Object[]> data, boolean flagBooleanValue, String... flags) {
        for (String flag : flags) {
            Set<String> findAllCases = stringUtil.findAllCases(flag);
            for (String flagName : findAllCases) {
                Object[] item = new Object[2];
                item[0] = flagName;
                item[1] = flagBooleanValue;
                data.add(item);
            }
        }
    }
}

Note: J’ai écrit la classe StringUtil et la méthode findAllCases() pour l’occasion, en TDD bien entendu. A partir du mot passé en paramètre, la méthode renvoie un Set de tous les combinaisons de casses possibles : start -> start, Start, STart, STArt, staRT, …

Les bienfaits des refactoring successifs

Le résultat est obtenu est désormais entièrement testé. De plus, la méthode asBoolean() accepte un paramètre dont la casse n’est plus importante.
Il suffit de trois étapes pour ajouter un nouveau flag, les étapes du cycle TDD :

1. Modifier l’un des deux appels dans la méthode createDataSet() de la classe de tests paramétrée. Ajoutons par exemple le flag stop en paramètre de la méthode addItem.

addItem(data, false, "no", "off", "stop");

Vérifier que les tests nouvellement générés échouent.

2. Ajouter le flag stop à la Map dans la classe StringSettingBool

private Map<String, Boolean> boolFlag = new HashMap<String, Boolean>() {
    {
        put("no", false);
        put("yes", true);
        put("off", false);
        put("on", true);
        put("start", true);
        put("stop", false);
    }
};

Vérifier que les tests nouvellement générés passent tous.

3. Refactorer si nécessaire

Que remarque-t-on ? Ajouter un nouveau flag prend désormais moins d’une minute ! Les risques d’erreur sont faibles (plus besoin de penser à ajouter une ligne par combinaison de casse). Le code est pleinement testé !

Conclusion

Ce premier billet consacré au TDD sur du code existant s’achève. Comme vous avez pu le constater, il est possible de se lancer dans l’apprentissage du TDD en prenant une partie de code peu complexe. Ayez toujours comme objectif d’avoir un code plus simple, plus court et mieux testé. Une nouvelle fonctionnalité devrait dorénavant être testable et implémentable plus facilement et plus rapidement.

Le prochain article sera consacré à l’extraction de dépendances et à l’utilisation de bouchons.

Published by

Publié par Julien Smadja

Julien Smadja est consultant manager chez Xebia. Il est également formateur au sein de Xebia Training .

Commentaire

4 réponses pour " Peut-on faire du TDD sur du code existant ? "

  1. Published by , Il y a 11 ans

    Sur un code plus complexe, il est très souvent possible de s’en sortir en appliquant le refactoring pattern « Extract Method Object ». Cela permet de cerner le code testé, quitte à regrouper ensuite les Method objects par affinité et relation dans de nouveaux objects.

  2. Published by , Il y a 11 ans

    Merci Olivier pour votre commentaire.

    En effet, le pattern « Extract Method Object » sera abordé dans un prochain article qui sera un peu plus complexe que celui-ci. Nous aborderons la notion d’extraction de dépendances et de Mocking.

    Stay tuned :-)

  3. Published by , Il y a 11 ans

    Article intéressant.

    Dans le cas présent, l’utilisation d’un test junit paramétré, n’est, à mon sens, pas la solution. On perd ici en lisibilité, et on perd l’intention du code : des flags équivalent Vrai, des flags équivalent Faux, et le cas des éléments n’ayant pas de correspondance.

    Un « custom assert » aurait été adapté.

    Il est à noté, que des legacy code retreat ont été lancés cet année, sur le sujet : http://whatis.legacycoderetreat.com.
    Les codes sources sont disponibles sur GitHub : https://github.com/jbrains/trivia.

    Jérémie.

  4. Published by , Il y a 11 ans

    C’est en effet la stratégie qu’il faut adopter pour éviter de modifier le comportement existant. Il me semble d’ailleurs que c’est la stratégie préconisé dans l’excellent « Working Effectively with Legacy Code » (de Michael Feathers).
    En pratique on est quelque fois confronté à des problemes de dépendences en particulier de violation du dependency inversion principle, j’attends donc la suite avec impatience :)

    PS: La méthode asBoolean() peut aussi s’écrire:
    public Boolean asBoolean(String flag) {
    if (flag == null) {
    return null;
    }
    return boolFlag.get(flag.toLowerCase());
    }

Laisser un commentaire

Votre adresse e-mail ne sera pas publiée. Les champs obligatoires sont indiqués avec *

Nous recrutons

Être un Sapient, c'est faire partie d'un groupe de passionnés ; C'est l'opportunité de travailler et de partager avec des pairs parmi les plus talentueux.