https://yattom.hatenablog.com/entry/20130109/p1

前回はテストのリファクタリングをしました。テストのリファクタリングについてはこちらもどうぞ。

リファクタリングの続きとして、TODOリストの「文字列のI/Oを別クラスにする」というのもやってしまおうと思います。これは前にも書いていましたが、ライフゲームの初期状態を文字列形式で与えたり、現在の状態を文字列で取り出したりする部分を別クラスにするという方針で行きましょう。

以下のテストケースでは、文字列のパターンを与えてGameOfLifeの内部構造を構築しています。

life_test.py

    def test_build_cells(self):
        initial = '''
            oo.
            o..
            ...
'''
        cells = GameOfLife.build_cells(pattern(initial))
        expected = {
            (0, 0): True,  (1, 0): True,  (2, 0): False,
            (0, 1): True,  (1, 1): False, (2, 1): False,
            (0, 2): False, (1, 2): False, (2, 2): False,
        }
        actual = { pos:cells[pos].is_alive() for pos in cells.keys() }
        self.assertEqual(actual, expected)

新しく作るクラスを使って、同じテストを書けそうです。書いてみましょう。新しいクラスはGameBuilderとします。

class GameBuilderTest(unittest.TestCase):
    def test_build_cells(self):
        initial = '''
            oo.
            o..
            ...
'''
        game = GameBuilder.build_with(initial)
        expected = {
            (0, 0): True,  (1, 0): True,  (2, 0): False,
            (0, 1): True,  (1, 1): False, (2, 1): False,
            (0, 2): False, (1, 2): False, (2, 2): False,
        }
        actual = { pos:game.cells[pos].is_alive() for pos in game.cells.keys() }
        self.assertEqual(actual, expected)

GameBuilder.build_with()でGameOfLifeオブジェクトが取れるようにしました。GoFのBuilderパターンを意識しているので、本来はインスタンスを作る → 構築する → 結果を取り出す、という3段階の操作をすべきところです。いっぽう使う側から見ると、メソッドひとつで結果が取り出せる方が簡単です。テストケースは使う側の視点から書き、設計はここから最適なものを導いていきましょう。必要に応じて、テストケースが増えるはずです。

実装も書いてみます。GameOfLife.build_cellsをコピーして作ってしまいましょう。GameOfLife側のbuild_cellsは、後で削除するつもりです。上で書いたテストを通すだけなので、GameOfLife自体はまだ変更しません。

life.py

class GameBuilder(object):
    @staticmethod
    def build_with(pattern):
        cells = GameBuilder.build_cells(pattern)

        GameOfLife.connect_neighbours(cells)
        game = GameOfLife()
        game.cells = cells

        return game

    @staticmethod
    def build_cells(pattern):
        cells = {}
        for y, line in enumerate(pattern.strip().split("\n")):
            for x, c in enumerate(line.strip()):
                if c == 'o':   state = Cell.ALIVE
                elif c == '.': state = Cell.DEAD
                cells[(x, y)] = Cell(state)
        return cells

build_cellsはそのままです。build_withを新たに書きましたが、GameOfLifeのコンストラクタ(__init__メソッド)を参考に、少々乱暴なやり方でGameOfLifeオブジェクトを構築しています。この部分、以下のように書き直します。

    def build_with(pattern):
        cells = GameBuilder.build_cells(pattern)
        GameBuilder.connect_neighbours(cells)
        return GameOfLife(cells=cells)

これに合わせて、さらにコードをリファクタリングします。こんどはGameOfLifeのコンストラクタを変更します。

class GameOfLife(object):
    def __init__(self, cells):
        self.cells = cells

...

class GameBuilder(object):
...
    NEIGHBOUR_POSITIONS = [(dx, dy) for dx in range(-1, 2) for dy in range(-1, 2) if not dx == dy == 0]

    @staticmethod
    def connect_neighbours(cells):
        for x, y in cells.keys():
           GameBuilder.connect_neighbour(cells[(x, y)], x, y, cells)

    @staticmethod
    def connect_neighbour(cell, x, y, cells):
        for dx, dy in GameBuilder.NEIGHBOUR_POSITIONS:
            if (x + dx, y + dy) not in cells: continue
            cell.neighbours.append(cells[(x + dx, y + dy)])

connect_neighboursをGameBuilderに移動しました。コンストラクタの引数も変わったためテストが大量に失敗するようになります。ここがリファクタリングの素敵なところで、失敗するテストをすべてグリーンにできれば、変更は完了です。残りあといくつかもわかるし、修正漏れがないか心配することもありません(テストをちゃんと書いていればです)。言語やIDEによってはこの種のリファクタリングを自動でやってくれるので、変更の手間はさらに軽減できます。

リファクタリングの過程で、以下のような変更をしました。

コンストラクタの引数変更に合わせて、テスト内でGameOfLifeを作っているところはGameBuilderを使うようにした
GameOfLifeTestの一部のテストケースをGameBuilderTestに移動した(build_cellsとconnect_neighboursのテスト)
結果的に、cellsを構築する処理はすべてGameBuilderに移り、GameOfLifeには次世代に遷移する処理が残った(dumpもまだ残っている)
GameOfLifeTest.test_set_initial_patternというテストケースが、実質的にGameBuilderのテストになり、しかも同じことをもっとちゃんとテストしているケースがあるので、削除した。

リファクタリングにより設計が改善され、各クラスの責務がはっきりしました。あわせてテストも整理した結果、テストを減らすこともできています。

同じ要領で、GameOfLife.dump()も別のクラスに移動します。GameDumperを作ってそちらにdump()の実装を移動した結果、GameOfLifeは純粋にライフゲームのロジックだけが残りました。

GameBuilderとGameDumperは文字列表現を扱いますが、他の表現(GUIとかJSONとか)も扱いたくなる想定をしています。そのときに備えて、汎用の処理と文字列専門の処理を別クラスに分けておいた方がいいかもしれません。おそらくGameBuilderはリファクタリングして汎用のロジックだけにして、文字列操作をGameStringBuilderのような名前のクラスにします。そうすれば例えば、GameJSONBuilderクラスを簡単に作れそうです。

しかし、今そう考えているだけなので、本当に最適解かどうかはやってみないとわかりません。本当の要求が見えていないので、今はこれ以上のリファクタリングはしないでおきます。必然性が証明されるまではリファクタリングをしてはいけません。「もっとかっこよくしたい」という欲求には、YAGNIのおまじないを唱えて耐えましょう。

今回はここまでとします。次回はとうとう、誕生以外のライフゲームのルールを実装します。コードはこちら https://github.com/yattom/tdd-life です。