Mokona Guu Center

Test Driven Development : du mouvement

Publié le

Je continue ma construction de programme implémentant les règles du Dôbutsu Shôgi en utilisant le Test Driven Development.

Cet article fait parti d'un ensemble dont voici les épisodes précédents :

Pièce de Shôgi en bois

Dans l'épisode précédent, j'avais créé une classe ShogiGame par extraction des méthodes de la classe de test ShogiGameTestCase. Une question légitime se présente : est-ce que ça ne vaudrait pas le coup d'extraire la classe dans son propre fichier ?

On peut. Se déplacer dans un même fichier est souvent plus simple et rapide que de passer d'un fichier à l'autre. Comme pour le moment, l'ensemble des tests et de la classe tient sur un écran (sur le mien en tout cas), je laisse les deux parties dans un même fichier.

Maintenant qu'on a la possibilité de mettre des pièces de jeu sur le plateau, j'ai envie de les déplacer.

Toujours dans l'esprit de réduire le test à sa plus simple expression, ma description sera constituée d'une coordonnée de départ et d'une coordonnée d'arrivée. Je test alors que le contenu est déplacé.

    def test_can_move_a_piece_on_the_board(self):
        def move(source, destination):
            pass

        PIECE_ELEPHANT = "E"
        PIECE_NONE = ""

        source = (0, 0)
        destination = (1, 1)

        self.game.drop(PIECE_ELEPHANT, source)

        move(source, destination)

        self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(destination))
        self.assertEqual(PIECE_NONE, self.game.get_piece_at(source))

Le test ne passe pas car move ne fait rien. Le système n'a encore aucun système pour gérer les coordonnées sur le plateau. Petit rappel : la première étape est de faire passer le test avec le minimum d'opérations possible, dans le test lui même.

    def test_can_move_a_piece_on_the_board(self):
        def move(source, destination):
            pass

        PIECE_ELEPHANT = "E"
        PIECE_NONE = ""

        source = (0, 0)
        destination = (1, 1)

        self.game.drop(PIECE_ELEPHANT, source)

        move(source, destination)

        self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(destination))
        self.game.dropped_piece = ""
        self.assertEqual(PIECE_NONE, self.game.get_piece_at(source))

Bon... on a respecté les règles, c'est valide, mais on n'est pas très avancé. Écrire un second test du même acabit ne va pas vraiment faire avancer. Le problème est que on ne peut pas toucher à la méthode get_piece_at, qui n'est pas une fonction locale.

Qu'à cela ne tienne.

    def test_can_move_a_piece_on_the_board(self):
        def move(source, destination):
            pass

        def get_piece_at(location):
            return self.game.get_piece_at(location)

        PIECE_ELEPHANT = "E"
        PIECE_NONE = ""

        source = (0, 0)
        destination = (1, 1)

        self.game.drop(PIECE_ELEPHANT, source)

        move(source, destination)

        self.assertEqual(PIECE_ELEPHANT, get_piece_at(destination))
        self.game.dropped_piece = ""
        self.assertEqual(PIECE_NONE, get_piece_at(source))

Nous voilà avec une fonction locale sur laquelle travailler. N'oublions pas, le test passe, nous sommes donc normalement en refactor.

Seulement, cette histoire de self.game.dropped_piece = \"\" nous met dans une impasse. Parfois, il faut revenir en arrière et chercher une autre solution.

    def test_can_move_a_piece_on_the_board(self):
        source = (0, 0)
        destination = (1, 1)

        def move(source, destination):
            pass

        def get_piece_at(location):
            if location == source:
                return PIECE_NONE
            if location == destination:
                return PIECE_ELEPHANT
            return self.game.get_piece_at(location)

        PIECE_ELEPHANT = "E"
        PIECE_NONE = ""

        self.game.drop(PIECE_ELEPHANT, source)

        move(source, destination)

        self.assertEqual(PIECE_ELEPHANT, get_piece_at(destination))
        self.assertEqual(PIECE_NONE, get_piece_at(source))

Voilà qui ressemble à quelque chose qui va dans la bonne direction. On voit les prémices d'une notion de position. En refactor, il n'y a pas vraiment d'autre chose à faire que de factoriser PIECE_ELEPHANT.

Cette pièce, je la déplace une fois. Pour continuer, mon test sera de la déplacer une seconde fois. Un autre test aurait pu être de déplacer une autre pièce. Mais dans ce cas, il aurait fallu se déplacer selon les mêmes coordonnées pour éviter de mélanger les concepts. Les tests nous auraient alors dirigé sur le concept des pièces de jeu plutôt que vers les mouvements.

Choisir les étapes qui amènent vers l'objectif est une partie importante du TDD. En choisissant le nouveau test, on influence sur la direction du code « généré ».

    def test_can_move_a_piece_twice_on_the_board(self):
        locations = []
        locations.append((0, 0))
        locations.append((1, 1))
        locations.append((2, 2))

        def move(source, destination):
            pass

        def get_piece_at(location):
            return self.game.get_piece_at(location)

        PIECE_NONE = ""

        self.game.drop(PIECE_ELEPHANT, locations[0])

        move(locations[0], locations[1])
        move(locations[1], locations[2])

        self.assertEqual(PIECE_ELEPHANT, get_piece_at(locations[2]))
        self.assertEqual(PIECE_NONE, get_piece_at(locations[0]))

Qui peut se résoudre avec cette implémentation de get_piece_at.

    def get_piece_at(location):
        if location == locations[0]:
            return PIECE_NONE
        if location == locations[2]:
            return PIECE_ELEPHANT
        return self.game.get_piece_at(location)

Avec maintenant deux implémentation de get_piece_at qui prennent en compte les positions, il est temps d'enlever les duplications et de travailler sur une implémentation plus générique.

Une résolution possible consiste à enregistrer les mouvements. Si plus tard on cherche une pièce qui est à la source d'un mouvement, on renvoie qu'il n'y a pas de pièce. Si la pièce est à une destination, on renvoie l'éléphant. C'est parfaitement faux dans le cas général, mais pour nos deux tests de déplacements, cela fonctionne :

    def move(source, destination):
        moves.append((source, destination))

    def get_piece_at(location):
        if location in [move[0] for move in moves]:
            return PIECE_NONE
        if location in [move[1] for move in moves]:
            return PIECE_ELEPHANT
        return self.game.get_piece_at(location)

move, get_piece_at et PIECE_NONE passent au niveau de la classe de tests.

Comme nous savons parfaitement que cette implémentation est très légère, écrivons un test concernant un mouvement qui ne passera pas.

Au passage, vous remarquerez les modifications de syntaxe des appels des méthodes au niveau de la classe de tests.

    def test_can_move_a_piece_twice_and_get_to_first_location(self):
        locations = []
        locations.append((0, 0))
        locations.append((1, 1))
        locations.append((0, 0))

        self.game.drop(PIECE_ELEPHANT, locations[0])

        self.move(locations[0], locations[1])
        self.move(locations[1], locations[2])

        self.assertEqual(PIECE_ELEPHANT, self.get_piece_at(locations[2]))
        self.assertEqual(PIECE_ELEPHANT, self.get_piece_at(locations[0]))

Fatalement, l'algorithme précédent ne fonctionne pas lorsque la pièce revient sur sa source. Mais nous ne pouvons pas corriger directement la fonction que nous avons déjà mis au niveau de la classe de tests ; une résolution doit être locale.

Techniquement, le test peut être résolu simplement : il suffit de faire systématiquement retourner à un get_piece_at local la valeur PIECE_ELEPHANT. J'ajoute le test self.assertEqual(PIECE_NONE, self.get_piece_at(locations[1])) pour que cette solution ne soit pas possible, car elle ne m'intéresse plus à ce niveau.

Je prépare aussi le terrain et réfléchissant à ce qu'il se passerait en continuant à résoudre les tests uniquement au travers de get_piece_at. Il y aurait un moyen en retraçant l'historique des mouvements à chaque get_piece_at. Si on dit qu'une optimisation prématurée est un mal, s'obliger à continuer dans une implémentation complexe et lente est aussi un mal.

Il est peut-être temps de tracer l'emplacement des pièces sur le plateau plus directement, et pour cela, il va y avoir besoin de toucher à drop. J'utilise la même méthode que précédemment pour rediriger une implémentation locale vers l'implémentation déjà extraite.

Un résolution possible est alors :

    def drop(piece, location):
        pieces_on_board[piece] = location
        self.game.drop(piece, location)

    def move(source, destination):
        pieces_at_source = [p[0] for p in pieces_on_board.items() if p[1] == source]
        if len(pieces_at_source) == 1:
            piece = pieces_at_source[0]
            pieces_on_board[piece] = destination
        else:
            self.move(source, destination)

    def get_piece_at(location):
        pieces_at_location = [p[0] for p in pieces_on_board.items() if p[1] == location]
        if len(pieces_at_location) == 1:
            piece = pieces_at_location[0]
            return piece
        else:
            return PIECE_NONE

Où on peut voir que j'ai gardé les appels aux fonctions déjà extraites sur les deux premières. Cela est nécessaire pour s'assurer qu'elles n'interfèrent pas avec la résolution local, cela aidera lors de l'extraction. Cependant, l'appel au get_piece_at extrait pose problème tel quel, je remplace donc par un return PIECE_NONE avant de chercher une solution.

Pour pouvoir extraire ces fonctions et les mélanger aux implémentations actuelles, appeler ces fonctions permet de préparer le terrain.

    def move(source, destination):
        pieces_at_source = [p[0] for p in pieces_on_board.items() if p[1] == source]
        if len(pieces_at_source) == 1:
            self.moves = []
            self.game.dropped_piece = PIECE_NONE
            piece = pieces_at_source[0]
            pieces_on_board[piece] = destination

    def get_piece_at(location):
        pieces_at_location = [p[0] for p in pieces_on_board.items() if p[1] == location]
        if len(pieces_at_location) == 1:
            piece = pieces_at_location[0]
            return piece
        else:
            return self.get_piece_at(location)

Afin de pouvoir appeler le get_piece_at extrait, je dois invalider son fonctionnement si move est appelé avec son nouveau fonctionnement. Ceci est fait en réinitialisant self.moves et self.game.dropped_piece. C'est un peu bancal, mais ça fonctionne, et c'est temporaire.

Je réinjecte cette solution dans le test précédent : le test passe toujours. Il s'agit donc maintenant d'extraire ces nouvelles implémentations au niveau de la classe de test.

C'est ici que l'appel aux fonctions extraites est intéressant, car l'extraction consiste à ajouter le code local au code extrait. Le code extrait remplace tout simplement l'appel à la méthode extraite.

Cela donne ceci :

    def move(self, source, destination):
        pieces_at_source = [p[0] for p in self.pieces_on_board.items() if p[1] == source]
        if len(pieces_at_source) == 1:
            self.moves = []
            self.game.dropped_piece = PIECE_NONE
            piece = pieces_at_source[0]
            self.pieces_on_board[piece] = destination
        else:
            self.moves.append((source, destination))

    def get_piece_at(self, location):
        pieces_at_location = [p[0] for p in self.pieces_on_board.items() if p[1] == location]
        if len(pieces_at_location) == 1:
            piece = pieces_at_location[0]
            return piece
        else:
            if location in [move[0] for move in self.moves]:
                return PIECE_NONE
            if location in [move[1] for move in self.moves]:
                return PIECE_ELEPHANT
            return self.game.get_piece_at(location)

    def drop(self, piece, location):
        self.pieces_on_board[piece] = location
        self.game.drop(piece, location)

C'est lourd et très redondant. Les tests passent, nous sommes donc toujours en phase de refactor. Avant de simplifier le tout, j'extrais encore ce code d'un niveau, vers la classe ShogiGame. Cela permettra de mettre tout le code au même endroit.

Rappel de la méthodologie : extraire le code en laissant les appels indirects en place. Puis transformer les appels de fonctions des tests vers les fonctions extraites. Puis enfin supprimer les fonctions de la classe de tests qui ne servaient plus qu'à rediriger les appels.

Ce qui donne (en oubliant capture_lion et has_winner) :

    class ShogiGame:
        def __init__(self):
            self.lion_was_captured = False
            self.dropped_piece = ""
            self.moves = []
            self.pieces_on_board = {}

        def drop(self, piece, location):
            self.pieces_on_board[piece] = location
            self.dropped_piece = piece

        def get_piece_at(self, location):
            pieces_at_location = [p[0] for p in self.pieces_on_board.items() if p[1] == location]
            if len(pieces_at_location) == 1:
                piece = pieces_at_location[0]
                return piece
            else:
                if location in [move[0] for move in self.moves]:
                    return PIECE_NONE
                if location in [move[1] for move in self.moves]:
                    return PIECE_ELEPHANT
                return self.dropped_piece

        def move(self, source, destination):
            pieces_at_source = [p[0] for p in self.pieces_on_board.items() if p[1] == source]
            if len(pieces_at_source) == 1:
                self.moves = []
                self.dropped_piece = PIECE_NONE
                piece = pieces_at_source[0]
                self.pieces_on_board[piece] = destination
            else:
                self.moves.append((source, destination))

Nous avons trois fonctionnements en parallèle : celui à base de dropped_piece, celui de moves et celui de pieces_on_board. Chacun des fonctionnements est normalement moins bon que le suivant, je vais donc les supprimer un par un, avec méthode.

Tout d'abord, dropped_piece se supprime en supprimant toutes ses assignations et en remplaçant return self.dropped_piece de get_piece_at par return PIECE_NONE.

Les tests passent.

Ensuite, moves se supprime aussi simplement en enlevant son initialisation dans __init__, tout le bloc else de get_piece_at, ne laissant que return PIECE_NONE. Et la partie else de move.

Les tests passent toujours.

À la chasse à la duplication toujours, j'ai deux fois un calcul identique pour pieces_at_source et pieces_at_location.

Voilà une possibilité :

    def _get_piece_at_location(self, location):
        pieces_at_location = [p[0] for p in self.pieces_on_board.items() if p[1] == location]
        if len(pieces_at_location) == 1:
            return pieces_at_location[0]
        return None

    def get_piece_at(self, location):
        piece = self._get_piece_at_location(location)
        return piece or PIECE_NONE

    def move(self, source, destination):
        piece = self._get_piece_at_location(source)
        if piece is not None:
            self.pieces_on_board[piece] = destination

Les tests nous assurent que cette factorisation ne change rien.

Il reste une duplication au niveau du mouvement de l'éléphant dans les tests. Car oui, la duplication se cherche aussi dans les tests.

Voici le résultat de cette séance. La totalité du fichier ne tient plus sur mon écran. Cependant, il reste toujours assez court pour me faire penser que son extraction rendrait moins pratique et rapide les refactors sans gain spécifique en contrepartie.

    import unittest

    PIECE_PAWN = "P"
    PIECE_ELEPHANT = "E"
    PIECE_NONE = ""

    class ShogiGame:
        def __init__(self):
            self.lion_was_captured = False
            self.pieces_on_board = {}

        def capture_lion(self):
            self.lion_was_captured = True

        def has_winner(self):
            return self.lion_was_captured

        def drop(self, piece, location):
            self.pieces_on_board[piece] = location

        def _get_piece_at_location(self, location):
            pieces_at_location = [p[0] for p in self.pieces_on_board.items() if p[1] == location]
            if len(pieces_at_location) == 1:
                return pieces_at_location[0]
            return None

        def get_piece_at(self, location):
            piece = self._get_piece_at_location(location)
            return piece or PIECE_NONE

        def move(self, source, destination):
            piece = self._get_piece_at_location(source)
            if piece is not None:
                self.pieces_on_board[piece] = destination


    class ShogiGameTestCase(unittest.TestCase):
        def setUp(self):
            self.game = ShogiGame()

        def test_has_no_winner_at_start(self):
            self.assertFalse(self.game.has_winner())

        def test_has_a_winner_if_a_lion_got_captured(self):
            self.game.capture_lion()
            self.assertTrue(self.game.has_winner())

        def test_has_a_dropped_pawn_on_its_board(self):
            location = (0, 0)
            self.game.drop(PIECE_PAWN, location)
            self.assertEqual(PIECE_PAWN, self.game.get_piece_at(location))

        def test_has_a_dropped_elephant_on_its_board(self):
            location = (0, 0)
            self.game.drop(PIECE_ELEPHANT, location)
            self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(location))

        def test_can_move_a_piece_on_the_board(self):
            source = (0, 0)
            destination = (1, 1)

            self.game.drop(PIECE_ELEPHANT, source)

            self.game.move(source, destination)

            self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(destination))
            self.assertEqual(PIECE_NONE, self.game.get_piece_at(source))

        def _move_elephant(self, locations):
            self.game.drop(PIECE_ELEPHANT, locations[0])
            self.game.move(locations[0], locations[1])
            self.game.move(locations[1], locations[2])

        def test_can_move_a_piece_twice_on_the_board(self):
            locations = []
            locations.append((0, 0))
            locations.append((1, 1))
            locations.append((2, 2))

            self._move_elephant(locations)

            self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(locations[2]))
            self.assertEqual(PIECE_NONE, self.game.get_piece_at(locations[0]))

        def test_can_move_a_piece_twice_and_get_to_first_location(self):
            locations = []
            locations.append((0, 0))
            locations.append((1, 1))
            locations.append((0, 0))

            self._move_elephant(locations)

            self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(locations[0]))
            self.assertEqual(PIECE_NONE, self.game.get_piece_at(locations[1]))
            self.assertEqual(PIECE_ELEPHANT, self.game.get_piece_at(locations[2]))