Mokona Guu Center

Test Driven Development : les pièces se changent

Publié le

J'ai beaucoup déplacé des pièces sur le tablier de Dôbutsu Shôgi jusqu'à maintenant. Pour le moment, il s'agit d'une implémentation où un joueur unique déplace des pièces un peu comme il le veut. Il peut aussi capturer des pièces et les parachuter.

J'aimerais à présent ajouter aux pièces les contraintes de mouvement. Pour cela, je voudrais commencer à nettoyer un peu pieces.py. Actuellement tout fonctionne avec des fonctions libres, chacune prenant en premier argument piece. Cela ressemble fortement à un classe.

Incidemment, utiliser une classe diminuerait le nombre de déclaration d'import à faire.

Mais comment transformer l'existant en utilisation d'objet ?

Pièce de Shôgi en bois

J'écris tout simplement ça :

    class Piece:
        pass

Et voilà !

Non... Il manque un truc !

On pourrait considérer que je suis en phase de refactor. Tous mes tests passent et je peux donc manipuler du code existant. Mais à vrai dire, l'existence d'une classe Piece est du nouveau code. Il me faut donc un nouveau test avant.

    class PieceTestCase(unittest.TestCase):
        def test_is_an_object(self):
            Piece()

Voilà. Maintenant je peux écrire la classe.

C'est un peu léger tout de même. Je voudrais créer une pièce spécifique et vérifier que mon implémentation équivalente présente des similitudes.

    class PieceTestCase(unittest.TestCase):
        def test_is_created_with_a_piece_type_and_a_player(self):
            piece = Piece(PAWN, P1)
            self.assertEqual(get_piece_controller(PIECE_PAWN_P1), piece.get_piece_controller())

Bien entendu, le test ne passe pas car, ni PAWN ni P1 n'existent, get_piece_controller() n'est pas une méthode de Piece et enfin le constructeur ne prend pour le moment pas de paramètres.

En toute honnêteté, c'est un peu trop de modifications d'un coup. Mais bon, le code existe déjà, je sais où je vais, je tente.

Note : je garde le nom de get_piece_controller() pour la méthode pour le moment afin de faciliter le déplacement de code. Cependant, à terme, le nom est redondant, je le transformerai en get_controller().

Après cette enjambée, je préfère repasser sur un baby step.

    class Piece:
        def __init__(self, piece_type, player):
            pass

        def get_piece_controller(self):
            return P1

Ok, ça passe. Je peux donc commencer une implémentation.

    class Piece:
        def __init__(self, piece_type, player):
            self.player = player

        def get_piece_controller(self):
            return self.player

À noter que je n'ai actuellement aucun besoin de récupérer le type de la pièce directement. Aucun code ne s'en sert. Je n'ajoute donc pas ce test.

Je teste l'équivalence suivante :

    class PieceTestCase(unittest.TestCase):
        def test_class_gives_the_same_opponent_as_function(self):
            piece = Piece(PAWN, P1)
            self.assertEqual(get_same_piece_controlled_by_opponent(PIECE_PAWN_P1), piece.get_same_piece_controlled_by_opponent())

Attention, je teste bien des équivalences. Je ne suis pas en train de ré-implementer la même chose, les tests qui ont servi à l'implémentation existent déjà.

Donc une solution est pour le moment :

    class Piece:
        def __init__(self, piece_type, player):
            self.piece_type = piece_type
            self.player = player

        def get_same_piece_controlled_by_opponent(self):
            return get_same_piece_controlled_by_opponent((self.piece_type, self.player))

Fonction suivante :

    class PieceTestCase(unittest.TestCase):
        def test_lion_status_is_the_same_as_with_the_function(self):
            pawn = Piece(PAWN, P1)
            self.assertEqual(is_a_lion(PIECE_PAWN_P1), pawn.is_a_lion())

            lion = Piece(LION, P1)
            self.assertEqual(is_a_lion(PIECE_LION_P1), lion.is_a_lion())

Toujours de l'équivalence. Solution facile :

    def is_a_lion(self):
        return is_a_lion((self.piece_type, self.player))

Ce qui me fait dire que tant qu'à se reposer sur le tuple que j'avais avant, autant avoir l'implémentation interne de Piece sous forme de tuple aussi.

    class Piece:
        def __init__(self, piece_type, player):
            self.piece = (piece_type, player)

        def get_piece_controller(self):
            return self.piece[1]

        def get_same_piece_controlled_by_opponent(self):
            return get_same_piece_controlled_by_opponent(self.piece)

        def is_a_lion(self):
            return is_a_lion(self.piece)

C'est plus simple et plus court. À vrai dire, je ne sais pas trop pourquoi je me suis embêté avec l'implémentation des deux variables membres. Probablement car n'ayant pas immédiatement besoin de piece_type, ça aurait été trop de code. J'aurais pu m'en appercevoir lors de l'implémentation de get_same_piece_controlled_by_opponent() en tant que méthode.

Reste la promotion. Vous avez compris :

    class Piece:
        def get_promoted_piece(self):
            return get_promoted_piece(self.piece)

    class PieceTestCase(unittest.TestCase):
        def test_promotion_is_the_same_as_with_the_function(self):
            pawn = Piece(PAWN, P1)
            self.assertEqual(get_promoted_piece(PIECE_PAWN_P1), pawn.get_promoted_piece())

            lion = Piece(LION, P1)
            self.assertEqual(get_promoted_piece(PIECE_LION_P1), lion.get_promoted_piece())

Ce que j'aimerais bien faire maintenant est de écrire mes constantes de pièces en tant que classe. Quelque chose comme ça :

    PIECE_PAWN_P1 = Piece("P", 1)

Je l'écris pour voir. D'un point de vue syntaxique, je dois déplacer les constantes et le dictionnaire des promotions après la définition de la classe. Puis je lance les tests.

La première erreur vient de l'absence de getitem() sur Piece. Cela vient du fait que j'essaie d'accéder à PIECE_PAWN_P1 comme si c'était un tuple.

Facile à corriger :

    class Piece:
        def __getitem__(self, index):
            return self.piece[index]

Le second problème est que la promotion ne fonctionne plus. En effet, le dictionnaire associe des tuples, qui sont des valeurs fixes et immuables. Cela ne fonctionne pas avec des objets instancié à différents endroits, comme c'est le cas ici :

    pawn = Piece(PAWN, P1)
    self.assertEqual(get_promoted_piece(PIECE_PAWN_P1), pawn.get_promoted_piece())

Mais il y a un moyen : fournir à Piece un moyen de comparer ses instances et de les placer dans un dictionnaire.

    class Piece:
        def __eq__(self, other):
            return self.piece == other.piece

        def __hash__(self):
            return 0

La valeur retournée par hash est très mauvaise, mais elle fonctionne et suffira pour le moment. eq défini la manière de comparer deux instances de Piece. Je fais ça en comparant leur représentation interne.

Je dois aussi changer la manière dont j’appelle la fonction de promotion :

    class Piece:
        def get_promoted_piece(self):
            return get_promoted_piece(self)

Pourtant, cela ne fonctionne toujours pas, car il se peut que je compare des tuples (ancienne représentation) et des objets (nouvelle représentation). Cela arrive avec la promotion sur ce passage :

    lion = Piece(LION, P1)
    self.assertEqual(get_promoted_piece(PIECE_LION_P1), lion.get_promoted_piece())

En effet, le lion n'étant pas promu, l'implémentation retourne un objet dans le cas de l'objet, et un tuple dans le cas de tuple ; c'est la fonction get() sur le dictionnaire qui veut ça.

J'implémente donc get_promoted_piece(self) comme ceci :

    class Piece:
        def get_promoted_piece(self):
            return promotions.get(self, self.piece)

Et voilà, les tests passent à nouveau. Au passage, j'ai fourni à Piece une représentation textuelle car j'en ai eu besoin pour tracer ce que je faisais à un moment.

    def __str__(self):
        return "Piece%s" % str(self.piece)

print PIECE_PAWN_P1 affiche donc à présent Piece('P', 1).

J'ajoute à présent une nouvelle pièce au nouveau système :

    PIECE_PAWN_P2 = Piece("P", 2)

Là, c'est le test self.assertEqual(PIECE_PAWN_P2, piece) qui ne fonctionne pas, car l'égalité entre un tuple et une Piece n'est pas définie.

Je redéfini la fonction pour que ça passe et faisant retourner à la fonction une instance de Piece.

    def get_same_piece_controlled_by_opponent(piece):
        controller = get_piece_controller(piece)
        return Piece(piece[0], _get_opponent_index(controller))

Ça passe.

Je continue à transformer les définitions de pièces en lançant les tests à chaque fois. Un seul ne passe pas, PIECE_LION_P1. À cause de self.assertEqual(get_promoted_piece(PIECE_LION_P1), lion.get_promoted_piece()).

Encore une fois, c'est la comparaison entre un tuple un instance de Piece qui ne passe pas. C'est le changement précédent de get_promoted_piece() qui pose problème. Mais à présent que toutes les autres pièces sont sous la nouvelle forme, cette modification n'est plus nécessaire, je peux écrire :

    def get_promoted_piece(self):
        return promotions.get(self, self)

Et voilà, tout passe.

Puisque j'ai des tests qui montrent (même s'ils ne prouvent pas) que les appels ancienne version et nouvelle version sont équivalent, je réécris les tests en version objet.

    class PieceTestCase(unittest.TestCase):
        def test_has_a_controller(self):
            self.assertEqual(PIECE_PAWN_P1.get_piece_controller(), PIECE_LION_P1.get_piece_controller())

        def test_can_have_a_different_controller(self):
            self.assertNotEqual(PIECE_PAWN_P2.get_piece_controller(), PIECE_PAWN_P1.get_piece_controller())

        def test_can_change_from_controller(self):
            piece = PIECE_PAWN_P1.get_same_piece_controlled_by_opponent()
            self.assertEqual(PIECE_PAWN_P2, piece)

        def test_can_be_controlled_by_player_1(self):
            self.assertEqual(1, PIECE_PAWN_P1.get_piece_controller())

        def test_can_be_piece_controlled_by_player_2(self):
            self.assertEqual(2, PIECE_ELEPHANT_P2.get_piece_controller())

        def test_has_a_special_none_value(self):
            self.assertEqual(0, PIECE_NONE.get_piece_controller())

        def test_can_be_a_lion(self):
            self.assertTrue(PIECE_LION_P1.is_a_lion())
            self.assertTrue(PIECE_LION_P2.is_a_lion())
            self.assertFalse(PIECE_PAWN_P1.is_a_lion())

        def test_can_promote_pawn_to_hen(self):
            self.assertEqual(PIECE_HEN_P1, PIECE_PAWN_P1.get_promoted_piece())
            self.assertEqual(PIECE_HEN_P2, PIECE_PAWN_P2.get_promoted_piece())

        def test_cannot_be_promoted_other_pieces(self):
            self.assertEqual(PIECE_LION_P1, PIECE_LION_P1.get_promoted_piece())

À chaque modification, je lance les tests pour vérifier que tout fonctionne toujours. Ce qui est le cas.

Je peux bouger le code des fonctions libres dans les méthodes de Piece.

Je réécris tout en nommant les constantes qu'il reste. Puis je lance l'intégralité des tests. Ceux de Board passent, mais ceux de ShogiGame ne passent pas, car il y a des appels aux fonctions qui ont été supprimées.

Je les écris dans le nouveau style.

Cependant, un test ne passe pas, à cause d'un problème dans Board. Le problème est dans get_piece_location() qui utilise effectivement une représentation interne des pièces. Forcément, ce n'est pas un code très robuste et il le fait savoir.

La correction est évidente, mais avant de la faire, j'ajoute un test à Board.

    def test_gives_a_valid_location_for_piece_on_board(self):
        location = (0, 0)
        self.board.place_piece(PIECE_LION_P1, location)
        location = self.board.get_piece_location(PIECE_LION_P1)
        self.assertEqual(INVALID_LOCATION, location)

Ça passe... zut. Mauvaise analyse. J'analyse un peu mieux l'erreur des tests de ShogiGame. J'enlève ce nouveau test, il ne sert à rien.

En fait,le problème est ici :

    def get_piece_location(self, piece):
        locations = [p[0] for p in self.pieces_on_board.items() if p[1] == piece]
        if len(locations) > 0:
            return locations[0]
        return INVALID_LOCATION

Dans les tests d'égalité de la création de la liste, il peut y avoir dans items() des valeurs None induites par les pièces enlevées par remove_piece().

J'ai deux choix. Soit je change Piece.eq() pour traiter None. Ça donnerait ça :

    def __eq__(self, other):
        return other is not None and self.piece == other.piece

Soit je me demande pourquoi j'utilise None dans Board plutôt que PIECE_NONE.

Je préfère le second choix pour éviter de surcharger le test d'égalité. Ce qui donne :

    class Board:
        def remove_piece(self, location):
            self.pieces_on_board[location] = PIECE_NONE

        def get_piece_at(self, location):
            return self.pieces_on_board.get(location, PIECE_NONE)

Cela simplifie le code de Board plutôt que de complexifié Piece. Ça va dans le bon sens.

Tout passe.

Je reviens donc sur Piece pour une séance de renommage de fonction. J'en profite pour renommer, sur les conseils de PYM, les fonctions d'ajout et de récupération de pièces dans Tray par push_piece() et pop_piece()/

Et donc, voici pieces.py, où **getitem**() et **str**() ont été enlevés, car un test de couverture de code m'ont indiqué que ces méthodes n'étaient plus utilisées :

    """ Defines all the needed game pieces. """

    PAWN = "P"
    LION = "L"
    ELEPHANT = "E"
    HEN = "H"
    P1 = 1
    P2 = 2


    def _get_opponent_index(player_index):
        return [0, 2, 1][player_index]


    class Piece:
        def __init__(self, piece_type, player):
            self.piece = (piece_type, player)

        def get_controller(self):
            return self.piece[1]

        def get_piece_controlled_by_opponent(self):
            controller = self.get_controller()
            return Piece(self.piece[0], _get_opponent_index(controller))

        def is_a_lion(self):
            return self.piece[0] == LION

        def get_promoted_piece(self):
            return promotions.get(self, self)

        def __eq__(self, other):
            return self.piece == other.piece

        def __hash__(self):
            return 0


    PIECE_PAWN_P1 = Piece(PAWN, P1)
    PIECE_PAWN_P2 = Piece(PAWN, P2)
    PIECE_ELEPHANT_P2 = Piece(ELEPHANT, P2)
    PIECE_LION_P1 = Piece(LION, P1)
    PIECE_LION_P2 = Piece(LION, P2)
    PIECE_HEN_P1 = Piece(HEN, P1)
    PIECE_HEN_P2 = Piece(HEN, P2)
    PIECE_NONE = Piece("", 0)

    promotions = {
        PIECE_PAWN_P1: PIECE_HEN_P1,
        PIECE_PAWN_P2: PIECE_HEN_P2,
    }

Et voilà

Cette session n'aura été qu'une transformation. Grâce à l'ensemble des tests, la modification se fait dans la confiance. Le harnais de sécurité nous rattrape dès que les changements sortent des clous.

Si se lancer dans le TDD semble très coûteux au début, si les premiers tests écrits ne sont pas forcément très bon, ce n'est que l'effet de l'apprentissage. Et les efforts payent. Lors des remaniements, lors des ajouts, lors des debugs.

C'est un investissement qui vaut l'effort initial.