Статьи

Рефакторинг Legacy Code: Часть 5 — Тестируемые методы игры

Старый код Гадкий код. Сложный код. Код спагетти. Глупый бред. В двух словах, Кодекс Наследия . Это серия, которая поможет вам работать и справляться с этим.

В нашем предыдущем уроке мы протестировали функции Runner. На этом уроке пришло время продолжить, где мы остановились, протестировав наш класс Game . Теперь, когда вы начинаете с такого большого куска кода, как у нас здесь, возникает соблазн начать тестирование сверху вниз, метод за методом. В большинстве случаев это невозможно. Гораздо лучше начать тестировать его короткими тестируемыми методами. Вот что мы будем делать на этом уроке: найдите и протестируйте эти методы.

Чтобы протестировать класс, нам нужно инициализировать объект этого конкретного типа. Мы можем считать, что наш первый тест — создать такой новый объект. Вы будете удивлены, сколько секретов могут скрыть конструкторы.

1
2
3
4
5
6
7
8
9
require_once __DIR__ .
 
class GameTest extends PHPUnit_Framework_TestCase {
 
    function testWeCanCreateAGame() {
        $game = new Game();
    }
 
}

К нашему удивлению, Game можно создать довольно легко. Нет проблем при запуске только new Game() . Ничего не ломается. Это очень хорошее начало, особенно если учесть, что конструктор Game довольно большой и делает много вещей.

Соблазнительно упростить конструктор прямо сейчас. Но у нас есть только золотой мастер, чтобы убедиться, что мы ничего не сломаем. Прежде чем перейти к конструктору, нам нужно протестировать большую часть остальной части класса. Итак, с чего нам начать?

Найдите первый метод, который возвращает значение, и спросите себя: «Могу ли я вызвать и контролировать возвращаемое значение этого метода?». Если ответ — да, это хороший кандидат для нашего теста.

1
2
3
4
function isPlayable() {
    $minimumNumberOfPlayers = 2;
    return ($this->howManyPlayers() >= $minimumNumberOfPlayers);
}

Как насчет этого метода? Кажется, хороший кандидат. Всего две строки, и он возвращает логическое значение. Но подождите, он вызывает другой метод, howManyPlayers() .

1
2
3
function howManyPlayers() {
    return count($this->players);
}

Это в основном просто метод, который подсчитывает элементы в массиве players класса. Хорошо, так что если мы не добавим игроков, оно должно быть равно нулю. isPlayable() должен вернуть false. Посмотрим, правильно ли наше предположение.

1
2
3
4
function testAJustCreatedNewGameIsNotPlayable() {
    $game = new Game();
    $this->assertFalse($game->isPlayable());
}

Мы переименовали наш предыдущий метод тестирования, чтобы отразить то, что мы действительно хотим проверить. Тогда мы просто утверждали, что игра не играбельна. Тест проходит. Но ложные срабатывания распространены во многих случаях. Так что для разума мы можем утверждать истину и убедиться, что тест не пройден.

1
$this->assertTrue($game->isPlayable());

И это делает!

1
2
PHPUnit_Framework_ExpectationFailedException :
Failed asserting that false is true.

Пока что довольно многообещающе. Нам удалось протестировать начальное возвращаемое значение метода, значение, представленное начальным состоянием класса Game . Пожалуйста, обратите внимание на подчеркнутое слово: «государство». Нам нужно найти способ контролировать состояние игры. Нам нужно его изменить, чтобы в нем было минимальное количество игроков.

Если мы проанализируем метод add() Game , мы увидим, что он добавляет элементы в наш массив.

1
array_push($this->players, $playerName);

Наше предположение подтверждается тем, как метод add() используется в RunnerFunctions.php .

1
2
3
4
5
6
7
8
9
function run() {
 
    $aGame = new Game();
    $aGame->add(«Chet»);
    $aGame->add(«Pat»);
    $aGame->add(«Sue»);
 
    // … //
}

Основываясь на этих наблюдениях, мы можем заключить, что дважды используя add() , мы сможем привести нашу Game в состояние с двумя игроками.

1
2
3
4
5
6
function testAfterAddingTwoPlayersToANewGameItIsPlayable() {
    $game = new Game();
    $game->add(‘First Player’);
    $game->add(‘Second Player’);
    $this->assertTrue($game->isPlayable());
}

Добавляя этот второй метод тестирования, мы можем гарантировать, что isPlayable() возвращает true, если выполняются условия.

Но вы можете подумать, что это не совсем модульный тест. Мы используем метод add() ! Мы используем больше, чем минимум кода. Вместо этого мы могли бы просто добавить элементы в массив $players Players и вообще не полагаться на метод add() .

Ну, ответ да и нет. Мы могли бы сделать это с технической точки зрения. Это будет иметь преимущество прямого контроля над массивом. Однако у него будет недостаток дублирования кода между кодом и тестами. Итак, выберите один из плохих вариантов, с которыми вы можете жить, и используйте его. Я лично предпочитаю повторно использовать методы, такие как add() .

Мы на зеленом, мы рефакторинг. Можем ли мы сделать наши тесты лучше? Ну да, мы можем. Мы могли бы трансформировать наш первый тест, чтобы проверить все условия нехватки игроков.

1
2
3
4
5
6
function testAGameWithNotEnoughPlayersIsNotPlayable() {
    $game = new Game();
    $this->assertFalse($game->isPlayable());
    $game->add(‘A player’);
    $this->assertFalse($game->isPlayable());
}

Возможно, вы слышали о понятии «одно утверждение на тест». Я в основном согласен с этим, но если у вас есть тест, который проверяет одну концепцию и требует несколько утверждений для ее проверки, я думаю, что допустимо использовать более одного утверждения. Эта точка зрения также активно поддерживается Робертом Мартином в его учениях.

Но как насчет нашего второго метода испытаний? Это достаточно хорошо? Я говорю нет.

1
2
$game->add(‘First Player’);
$game->add(‘Second Player’);

Эти два звонка меня немного беспокоят. Они являются подробной реализацией без явного объяснения в нашем методе. Почему бы не извлечь их в приватный метод?

01
02
03
04
05
06
07
08
09
10
function testAfterAddingEnoughPlayersToANewGameItIsPlayable() {
    $game = new Game();
    $this->addEnoughPlayers($game);
    $this->assertTrue($game->isPlayable());
}
 
private function addEnoughPlayers($game) {
    $game->add(‘First Player’);
    $game->add(‘Second Player’);
}

Это намного лучше, и это также приводит нас к другой концепции, которую мы пропустили. В обоих тестах мы так или иначе выражали понятие «достаточно игроков». Но сколько достаточно? Это два? Да, пока это так. Но хотим ли мы, чтобы наш тест провалился, если для логики игры потребуется не менее трех игроков? Мы не хотим, чтобы это произошло. Мы можем ввести для него открытое статическое поле класса.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
class Game {
    static $minimumNumberOfPlayers = 2;
 
    // … //
 
    function __construct() {
        // … //
    }
 
    function isPlayable() {
        return ($this->howManyPlayers() >= self::$minimumNumberOfPlayers);
    }
 
    // … //
}

Это позволит нам использовать его в наших тестах.

1
2
3
4
5
private function addEnoughPlayers($game) {
    for($i = 0; $i < Game::$minimumNumberOfPlayers; $i++) {
        $game->add(‘A Player’);
    }
}

Наш маленький вспомогательный метод будет просто добавлять игроков, пока не будет добавлено достаточно. Мы даже можем создать еще один такой метод для нашего первого теста, поэтому мы добавим почти достаточно игроков.

01
02
03
04
05
06
07
08
09
10
11
12
function testAGameWithNotEnoughPlayersIsNotPlayable() {
    $game = new Game();
    $this->assertFalse($game->isPlayable());
    $this->addJustNothEnoughPlayers($game);
    $this->assertFalse($game->isPlayable());
}
 
private function addJustNothEnoughPlayers($game) {
    for($i = 0; $i < Game::$minimumNumberOfPlayers — 1; $i++) {
        $game->add(‘A player’);
    }
}

Но это привело к некоторому дублированию. Наши два вспомогательных метода довольно похожи. Разве мы не можем извлечь третий из них?

01
02
03
04
05
06
07
08
09
10
11
12
13
private function addEnoughPlayers($game) {
    $this->addManyPlayers($game, Game::$minimumNumberOfPlayers);
}
 
private function addJustNothEnoughPlayers($game) {
    $this->addManyPlayers($game, Game::$minimumNumberOfPlayers — 1);
}
 
private function addManyPlayers($game, $numberOfPlayers) {
    for ($i = 0; $i < $numberOfPlayers; $i++) {
        $game->add(‘A Player’);
    }
}

Это лучше, но это создает другую проблему. Мы сократили дублирование в этих методах, но наш объект $game теперь проходит три уровня. Становится трудно управлять. Пришло время инициализировать его в setUp() теста и использовать его повторно.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
class GameTest extends PHPUnit_Framework_TestCase {
 
    private $game;
 
    function setUp() {
        $this->game = new Game;
    }
 
    function testAGameWithNotEnoughPlayersIsNotPlayable() {
        $this->assertFalse($this->game->isPlayable());
        $this->addJustNothEnoughPlayers();
        $this->assertFalse($this->game->isPlayable());
    }
 
    function testAfterAddingEnoughPlayersToANewGameItIsPlayable() {
        $this->addEnoughPlayers($this->game);
        $this->assertTrue($this->game->isPlayable());
    }
 
    private function addEnoughPlayers() {
        $this->addManyPlayers(Game::$minimumNumberOfPlayers);
    }
 
    private function addJustNothEnoughPlayers() {
        $this->addManyPlayers(Game::$minimumNumberOfPlayers — 1);
    }
 
    private function addManyPlayers($numberOfPlayers) {
        for ($i = 0; $i < $numberOfPlayers; $i++) {
            $this->game->add(‘A Player’);
        }
    }
 
}

Намного лучше. Весь нерелевантный код находится в закрытых методах, $game инициализируется в setUp() и многие методы были удалены из тестовых методов. Однако здесь нам пришлось пойти на компромисс. В нашем первом тесте мы начнем с утверждения. Это предполагает, что setUp() всегда будет создавать пустую игру. Это нормально на данный момент. Но, в конце концов, вы должны понимать, что идеального кода не существует. Есть просто код с компромиссами, с которыми вы готовы жить.

Если мы сканируем наш класс Game сверху вниз, следующий метод в нашем списке — add() . Да, тот же метод, который мы использовали в наших тестах в предыдущем абзаце. Но можем ли мы это проверить?

1
2
3
4
function testItCanAddANewPlayer() {
    $this->game->add(‘A player’);
    $this->assertEquals(1, count($this->game->players));
}

Теперь это другой способ тестирования объектов. Мы вызываем наш метод и затем проверяем состояние объекта. Поскольку add() всегда возвращает true , мы не можем проверить его вывод. Но мы можем начать с пустого объекта Game и затем проверить, есть ли один пользователь после того, как мы добавим его. Но достаточно ли проверки?

1
2
3
4
5
function testItCanAddANewPlayer() {
    $this->assertEquals(0, count($this->game->players));
    $this->game->add(‘A player’);
    $this->assertEquals(1, count($this->game->players));
}

Не лучше ли также проверить, нет ли игроков, прежде чем мы вызовем add() ? Ну, это может быть немного слишком много здесь, но, как вы можете видеть из кода выше, мы могли бы сделать это. И всякий раз, когда вы не уверены в исходном состоянии, вы должны сделать на нем утверждение. Это также защищает вас от будущих изменений кода, которые могут изменить исходное состояние вашего объекта.

Но мы тестируем все, что делает метод add() ? Я говорю нет. Помимо добавления пользователя, он также устанавливает множество настроек для него. Мы также должны проверить это.

1
2
3
4
5
6
7
8
function testItCanAddANewPlayer() {
    $this->assertEquals(0, count($this->game->players));
    $this->game->add(‘A player’);
    $this->assertEquals(1, count($this->game->players));
    $this->assertEquals(0, $this->game->places[1]);
    $this->assertEquals(0, $this->game->purses[1]);
    $this->assertFalse($this->game->inPenaltyBox[1]);
}

Это лучше. Мы проверяем каждое действие, которое выполняет метод add() . На этот раз я предпочел напрямую протестировать массив $players Players. Почему? Мы могли бы использовать метод howManyPlayers() который делает то же самое, верно? Что ж, в этом случае мы посчитали, что более важно описать наши утверждения эффектами, которые метод add() оказывает на состояние объекта. Если нам нужно изменить add() , мы ожидаем, что тест, проверяющий его строгое поведение, потерпит неудачу. У меня были бесконечные дебаты с моими коллегами в Syneto по этому поводу. Тем более, что этот тип тестирования представляет сильную связь между тестом и тем, как на самом деле реализован метод add() . Так что, если вы предпочитаете проверять это наоборот, это не значит, что ваши идеи ошибочны.

Мы можем спокойно игнорировать проверку вывода, echoln() . Они просто выводят контент на экран. Мы пока не хотим трогать эти методы. Наш золотой мастер полностью полагается на этот результат.

У нас есть еще один проверенный метод с новым проходным тестом. Пришло время немного изменить их. Давайте начнем с наших тестов. Не слишком ли запутаны последние три утверждения? Похоже, они не связаны строго с добавлением игрока. Давайте изменим это:

1
2
3
4
5
6
function testItCanAddANewPlayer() {
    $this->assertEquals(0, count($this->game->players));
    $this->game->add(‘A player’);
    $this->assertEquals(1, count($this->game->players));
    $this->assertDefaultPlayerParametersAreSetFor(1);
}

Так-то лучше. Этот метод теперь более абстрактный, многократно используемый, с выразительным названием и скрывает все несущественные детали.

Мы можем сделать что-то подобное с нашим рабочим кодом.

1
2
3
4
5
6
7
8
function add($playerName) {
    array_push($this->players, $playerName);
    $this->setDefaultPlayerParametersFor($this->howManyPlayers());
 
    echoln($playerName . » was added»);
    echoln(«They are player number » . count($this->players));
    return true;
}

Мы извлекли неважные детали в setDefaultPlayerParametersFor() .

1
2
3
4
5
private function setDefaultPlayerParametersFor($playerId) {
    $this->places[$playerId] = 0;
    $this->purses[$playerId] = 0;
    $this->inPenaltyBox[$playerId] = false;
}

На самом деле эта идея пришла ко мне после того, как я написал тест. Это еще один хороший пример того, как тесты заставляют нас думать о нашем коде с другой точки зрения. Это другой взгляд на проблему, это то, что мы должны использовать и позволить нашим тестам руководить нашим дизайном рабочего кода.

Давайте найдем нашего третьего кандидата на тестирование. howManyPlayers() слишком прост и косвенно уже протестирован. roll() слишком сложен для непосредственного тестирования. Плюс это возвращает null . askQuestions() кажется интересным с первого взгляда, но это все представление, без возвращаемого значения.

currentCategory() является тестируемым, но его довольно сложно протестировать. Это огромный селектор с десятью условиями. Нам нужен тест длиной в десять строк, а затем мы должны серьезно реорганизовать этот метод и, конечно же, тесты. Мы должны принять к сведению этот метод и вернуться к нему после того, как мы закончили с более простыми. Для нас это будет в нашем следующем уроке.

wasCorrectlyAnswered() снова усложняется. Нам нужно извлечь из него небольшие кусочки кода, которые можно тестировать. Однако wrongAnswer() кажется многообещающим. Он выводит материал на экран, но также изменяет состояние нашего объекта. Давайте посмотрим, сможем ли мы это контролировать и протестировать.

1
2
3
4
5
6
function testWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox() {
    $this->game->add(‘A player’);
    $this->game->currentPlayer = 0;
    $this->game->wrongAnswer();
    $this->assertTrue($this->game->inPenaltyBox[0]);
}

Grrr … было довольно сложно написать этот метод испытаний. wrongAnswer() полагается на $this->currentPlayer для своей поведенческой логики, но он также использует $this->players в своей части презентации. Один уродливый пример того, почему вы не должны смешивать логику и представление. Мы разберемся с этим в следующем уроке. На данный момент мы проверили, что пользователь вводит штрафной ящик. Мы также должны заметить, что в методе есть оператор if() . Это условие, которое мы еще не тестировали, поскольку у нас есть только один игрок, и поэтому мы не выполняем условие. Мы могли бы проверить окончательное значение $currentPlayer . Но добавление этой строки кода в тест провалит его.

1
$this->assertEquals(1, $this->game->currentPlayer);

Более внимательный взгляд на закрытый метод shouldResetCurrentPlayer() выявляет проблему. Если индекс текущего игрока равен количеству игроков, он будет сброшен в ноль. Aaaahhh! Мы на самом деле вводим if() !

01
02
03
04
05
06
07
08
09
10
11
12
13
14
function testWhenAPlayerEntersAWrongAnswerItIsSentToThePenaltyBox() {
    $this->game->add(‘A player’);
    $this->game->currentPlayer = 0;
    $this->game->wrongAnswer();
    $this->assertTrue($this->game->inPenaltyBox[0]);
    $this->assertEquals(0, $this->game->currentPlayer);
}
 
function testCurrentPlayerIsNotResetAfterWrongAnswerIfOtherPlayersDidNotYetPlay() {
    $this->addManyPlayers(2);
    $this->game->currentPlayer = 0;
    $this->game->wrongAnswer();
    $this->assertEquals(1, $this->game->currentPlayer);
}

Хорошо. Мы создали второй тест, чтобы проверить конкретный случай, когда есть еще игроки, которые не играли. Нас не волнует состояние inPenaltyBox для второго теста. Нас интересует только индекс текущего игрока.

Последний метод, который мы можем протестировать, а затем рефакторинг — это didPlayerWin() .

1
2
3
4
function didPlayerWin() {
    $numberOfCoinsToWin = 6;
    return !($this->purses[$this->currentPlayer] == $numberOfCoinsToWin);
}

Мы можем сразу заметить, что его структура кода очень похожа на isPlayable() , метод, который мы тестировали в первую очередь. Наше решение должно быть чем-то похожим. Когда ваш код такой короткий, всего две-три строки, выполнение более чем одного крошечного шага — не такой большой риск. В худшем случае вы возвращаете три строки кода. Итак, давайте сделаем это за один шаг.

1
2
3
4
5
function testTestPlayerWinsWithTheCorrectNumberOfCoins() {
    $this->game->currentPlayer = 0;
    $this->game->purses[0] = Game::$numberOfCoinsToWin;
    $this->assertTrue($this->game->didPlayerWin());
}

Но ждать! Это не удается. Как это возможно? Разве это не должно пройти? Мы предоставили правильное количество монет. Если мы изучим наш метод, мы обнаружим небольшой вводящий в заблуждение факт.

1
return !($this->purses[$this->currentPlayer] == $numberOfCoinsToWin);

Возвращаемое значение фактически отрицается. Таким образом, метод не говорит нам, если игрок выиграл, он говорит нам, если игрок не выиграл игру. Мы могли бы пойти и найти места, где используется этот метод, и отрицать его ценность там. Затем измените его поведение здесь, чтобы не ложно отрицать ответ. Но он используется в wasCorrectlyAnswered() , метод, который мы пока не можем тестировать. Может быть, пока достаточно простого переименования, чтобы выделить правильную функциональность.

1
2
3
function didPlayerNotWin() {
    return !($this->purses[$this->currentPlayer] == self::$numberOfCoinsToWin);
}

Так что это о завершение учебника. Хотя нам не нравится отрицание в имени, это компромисс, который мы можем сделать на данном этапе. Это имя обязательно изменится, когда мы начнем рефакторинг других частей кода. Кроме того, если вы посмотрите на наши тесты, они выглядят странно сейчас:

1
2
3
4
5
function testTestPlayerWinsWithTheCorrectNumberOfCoins() {
    $this->game->currentPlayer = 0;
    $this->game->purses[0] = Game::$numberOfCoinsToWin;
    $this->assertFalse($this->game->didPlayerNotWin());
}

Протестировав false на отрицательном методе со значением, которое указывает на истинный результат, мы внесли много путаницы в читаемость наших кодов. Но сейчас это хорошо, так как нам нужно остановиться в какой-то момент, верно?

В нашем следующем уроке мы начнем работать над некоторыми из более сложных методов в классе Game . Спасибо за чтение.