Статьи

Рефакторинг устаревшего кода: часть 9 — анализ проблем

В этом уроке мы продолжим фокусироваться на нашей бизнес-логике. Мы RunnerFunctions.php принадлежит ли RunnerFunctions.php классу, и если да, то к какому классу? Мы будем думать о проблемах и методах. Наконец, мы узнаем немного больше о концепции насмешек. Чего же ты ждешь? Читай дальше.


Несмотря на то, что большая часть нашего кода представлена ​​в объектно-ориентированной форме, хорошо организованной по классам, некоторые функции просто размещаются в файле. Нам нужно взять некоторые из них, чтобы дать функциям RunnerFunctions.php более объектно-ориентированный аспект.

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
const WRONG_ANSWER_ID = 7;
const MIN_ANSWER_ID = 0;
const MAX_ANSWER_ID = 9;
 
function isCurrentAnswerCorrect($minAnswerId = MIN_ANSWER_ID, $maxAnswerId = MAX_ANSWER_ID) {
    return rand($minAnswerId, $maxAnswerId) != WRONG_ANSWER_ID;
}
 
function run() {
    $display = new CLIDisplay();
    $aGame = new Game($display);
    $aGame->add(«Chet»);
    $aGame->add(«Pat»);
    $aGame->add(«Sue»);
 
    do {
        $dice = rand(0, 5) + 1;
        $aGame->roll($dice);
    } while (!didSomebodyWin($aGame, isCurrentAnswerCorrect()));
}
 
function didSomebodyWin($aGame, $isCurrentAnswerCorrect) {
    if ($isCurrentAnswerCorrect) {
        return !
    } else {
        return !
    }
}

Мой первый инстинкт — просто заворачивать их в класс. В этом нет ничего гениального, но это то, что заставляет нас начать что-то менять. Давайте посмотрим, может ли идея действительно работать.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
const WRONG_ANSWER_ID = 7;
const MIN_ANSWER_ID = 0;
const MAX_ANSWER_ID = 9;
 
class RunnerFunctions {
 
    function isCurrentAnswerCorrect($minAnswerId = MIN_ANSWER_ID, $maxAnswerId = MAX_ANSWER_ID) {
        return rand($minAnswerId, $maxAnswerId) != WRONG_ANSWER_ID;
    }
 
    function run() {
        // … //
    }
 
    function didSomebodyWin($aGame, $isCurrentAnswerCorrect) {
        // … //
    }
}

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

1
2
3
4
5
6
7
8
private function generateOutput($seed) {
    ob_start();
    srand($seed);
    (new RunnerFunctions())->run();
    $output = ob_get_contents();
    ob_end_clean();
    return $output;
}

В нашем файле GoldenMasterTest.php мы должны изменить способ запуска нашего кода. Функция — generateOutput() и ее третью строку необходимо изменить, чтобы создать новый объект и вызвать для него run() . Но это не удается.

1
PHP Fatal error: Call to undefined function didSomebodyWin() in …

Теперь нам нужно изменить наш новый класс дальше.

1
2
3
4
do {
    $dice = rand(0, 5) + 1;
    $aGame->roll($dice);
} while (!$this->didSomebodyWin($aGame, $this->isCurrentAnswerCorrect()));

Нам нужно было только изменить условие оператора while в методе run() . Новый код вызывает didSomebodyWin() и isCurrentAnswerCorrect() из текущего класса, добавляя к ним $this-> .

Это делает золотой мастерский пас, но тормозит тесты бегуна.

1
PHP Fatal error: Call to undefined function isCurrentAnswerCorrect() in /…/RunnerFunctionsTest.php on line 25

Проблема в assertAnswersAreCorrectFor() , но ее легко исправить, создав сначала объект бегуна.

1
2
3
4
5
6
private function assertAnswersAreCorrectFor($correctAnserIDs) {
    $runner = new RunnerFunctions();
    foreach ($correctAnserIDs as $id) {
        $this->assertTrue($runner->isCurrentAnswerCorrect($id, $id));
    }
}

Эту же проблему необходимо решить и в трех других функциях.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
function testItCanFindWrongAnswer() {
    $runner = new RunnerFunctions();
    $this->assertFalse($runner->isCurrentAnswerCorrect(WRONG_ANSWER_ID, WRONG_ANSWER_ID));
}
 
function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided() {
    $runner = new RunnerFunctions();
    $this->assertTrue($runner->didSomebodyWin($this->aFakeGame(), $this->aCorrectAnswer()));
}
 
function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided() {
    $runner = new RunnerFunctions();
    $this->assertFalse($runner->didSomebodyWin($this->aFakeGame(), $this->aWrongAnswer()));
}

В то время как это делает код проходным, оно вводит немного дублирования кода. Теперь, когда у нас есть все тесты на зеленом, мы можем извлечь создание бегуна в метод 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
private $runner;
 
function setUp() {
    $this->runner = new Runner();
}
 
function testItCanFindCorrectAnswer() {
    $this->assertAnswersAreCorrectFor($this->getCorrectAnswerIDs());
}
 
function testItCanFindWrongAnswer() {
    $this->assertFalse($this->runner->isCurrentAnswerCorrect(WRONG_ANSWER_ID, WRONG_ANSWER_ID));
}
 
function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided() {
    $this->assertTrue($this->runner->didSomebodyWin($this->aFakeGame(), $this->aCorrectAnswer()));
}
 
function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided() {
    $this->assertFalse($this->runner->didSomebodyWin($this->aFakeGame(), $this->aWrongAnswer()));
}
 
private function assertAnswersAreCorrectFor($correctAnserIDs) {
    foreach ($correctAnserIDs as $id) {
        $this->assertTrue($this->runner->isCurrentAnswerCorrect($id, $id));
    }
}

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

Если вы не отметили « Поиск вхождений текста » в поле выше, не забудьте вручную изменить ваши включения, поскольку рефакторинг также переименует файл.

Теперь у нас есть файл с именем GameRunner.php , еще один с именем Runner.php и третий с именем Game.php . Я не знаю о вас, но это кажется мне очень запутанным. Если бы я впервые увидел в жизни эти три файла, я бы не знал, какой из них что делает. Нам нужно избавиться хотя бы от одного из них.

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

1
2
3
require_once __DIR__ .
require_once __DIR__ .
(new Runner())->run();

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


Я считаю, что одной из наиболее важных характеристик объектно-ориентированного программирования является определение проблем. Я всегда задаю себе вопросы типа: «Этот класс делает то, что говорит его имя?», «Является ли этот метод заботы об этом объекте?», «Должен ли мой объект заботиться об этой конкретной ценности?

Удивительно, но эти типы вопросов имеют большое значение для разъяснения как бизнес-предметной области, так и архитектуры программного обеспечения. Мы задаем такие вопросы и отвечаем на них в группе в Синето. Много раз, когда у программиста возникает дилемма, он или она просто встает, просит у команды две минуты внимания, чтобы узнать наше мнение по этому вопросу. Те, кто знаком с архитектурой кода, ответят с программной точки зрения, в то время как другие, более знакомые с бизнес-областью, могут пролить свет на некоторые важные идеи о коммерческих аспектах.

Давайте попробуем подумать о проблемах в нашем случае. Мы можем продолжать уделять внимание классу Runner . Исключить или трансформировать этот класс гораздо вероятнее, чем Game .

Во-первых, должен ли бегун заботиться о том, как isCurrentAnswerCorrect() ? Должен ли бегун иметь какие-либо знания о вопросах и ответах?

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

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

01
02
03
04
05
06
07
08
09
10
11
12
13
require_once __DIR__ .
include_once __DIR__ .
 
class Runner {
 
    function run() {
        //…//
    }
 
    function didSomebodyWin($aGame, $isCurrentAnswerCorrect) {
        //…//
    }
}

Важно отметить, что исчез не только метод, но и константа, определяющая пределы ответа.

Но как насчет didSomebodyWin() ? Должен ли бегун решать, когда кто-то победил? Если мы посмотрим на тело метода, то увидим проблему с подсветкой, как фонарик в темноте.

1
2
3
4
5
6
7
function didSomebodyWin($aGame, $isCurrentAnswerCorrect) {
    if ($isCurrentAnswerCorrect) {
        return !$aGame->wasCorrectlyAnswered();
    } else {
        return !$aGame->wrongAnswer();
    }
}

Что бы ни делал этот метод, он делает это только на объекте Game . Он проверяет текущий ответ, возвращаемый игрой. Затем он возвращает то, что игровой объект возвращает в своих wasCorrectlyAnswered() или wrongAnswer() . Этот метод сам по себе ничего не делает. Все, что его волнует, это Game . Это классический пример запаха кода под названием Feature Envy . Класс делает то, что должен делать другой класс. Время переместить это.

1
2
3
4
5
6
7
8
9
class RunnerFunctionsTest extends PHPUnit_Framework_TestCase {
 
    private $runner;
 
    function setUp() {
        $this->runner = new Runner();
    }
 
}

Как обычно, мы сначала перенесли тесты. TDD? Кто-нибудь?

В результате у нас больше не будет тестов, поэтому этот файл можно запустить. Удаление — моя любимая часть программирования.

И когда мы запускаем наши тесты, мы получаем приятную ошибку.

1
Fatal error: Call to undefined method Game::didSomebodyWin()

Теперь пришло время изменить код. Копирование и вставка метода в Game волшебным образом сделает все тесты успешными. И старые, и те переехали в GameTest . Но хотя это ставит метод в нужное место, у него есть две проблемы: нужно изменить и бегуна, и мы отправляем поддельный объект Game который нам больше не нужен, поскольку он является частью Game .

1
2
3
4
do {
    $dice = rand(0, 5) + 1;
    $aGame->roll($dice);
} while (!$aGame->didSomebodyWin($aGame, $this->isCurrentAnswerCorrect()));

Исправить бегун очень легко. Мы просто изменим $this->didSomebodyWin(...) на $aGame->didSomebodyWin(...) . Нам нужно будет вернуться сюда и снова изменить его после нашего следующего шага. Тест рефакторинг.

1
2
3
4
5
function testItCanTellIfThereIsNoWinnerWhenACorrectAnswerIsProvided() {
    $aGame = \Mockery::mock(‘Game[wasCorrectlyAnswered]’);
    $aGame->shouldReceive(‘wasCorrectlyAnswered’)->once()->andReturn(false);
    $this->assertTrue($aGame->didSomebodyWin($this->aCorrectAnswer()));
}

Пришло время для насмешек! Вместо использования нашего поддельного класса, определенного в конце наших тестов, мы будем использовать Mockery . Это позволяет нам легко переписать метод в Game , ожидать, что он будет вызван, и вернуть желаемое значение. Конечно, мы могли бы сделать это, заставив наш поддельный класс расширить Game и переписать метод самостоятельно. Но почему работа, для которой существует инструмент?

1
2
3
4
5
function testItCanTellIfThereIsNoWinnerWhenAWrongAnswerIsProvided() {
    $aGame = \Mockery::mock(‘Game[wrongAnswer]’);
    $aGame->shouldReceive(‘wrongAnswer’)->once()->andReturn(true);
    $this->assertFalse($aGame->didSomebodyWin($this->aWrongAnswer()));
}

После того, как наш второй метод был переписан, мы можем избавиться от фальшивого игрового класса и любых методов, которые его инициализировали. Проблемы решены!

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

Спасибо за уделенное время.