Старый код Гадкий код. Сложный код. Код спагетти. Глупый бред. В двух словах, Кодекс Наследия . Это серия, которая поможет вам работать и справляться с этим.
В наших предыдущих пяти уроках мы потратили довольно много времени на понимание нашей унаследованной системы, на написание тестов для любого тестируемого фрагмента кода, который мы могли найти. Мы достигли точки, где у нас есть немало проверенных методов, но мы все же избежали сложной, трудной для понимания логики. Настало время для серьезного кодирования.
Понимание метода roll()
Наш первый кандидат — метод roll() . Поскольку он не возвращает значения, кажется, неясно, что он делает и как его проверить. Когда я не уверен, как начать тестировать кусок кода, я стараюсь читать его построчно и понимать его шаг за шагом. Иногда это выполнимо, иногда код слишком сложен.
Находясь в процессе чтения и изучения, я также провожу небольшой рефакторинг, который, как я знаю, мой IDE может выполнять безопасно. В основном это переименования переменных и методов, которые, я думаю, я понимаю, и я хочу сделать их более очевидными для меня и любого будущего читателя. И как вторая мысль, наш золотой мастер все еще там для случайных испытаний.
Глядя на сигнатуру метода: roll($roll) , мне интересно, что означает параметр $roll ? Это объект? Это действие? Это число? Моя IDE может быть полезна здесь. Просто поместив курсор на параметр $roll , все его применения будут слегка выделены голубоватым цветом.
Мы можем наблюдать выделенные переменные $roll в строках 63, 67, 71. И это только вхождения, которые помещаются на экране. Хотя ниже может быть несколько других применений, эти три являются хорошими кандидатами, чтобы помочь нам выяснить роль переменной $roll .
В строке 63 он используется для печати текста на экране. echoln("They have rolled a " . $roll); , Эта строка на самом деле довольно проста для понимания, и она действительно полезна для нас. Это говорит нам о том, что какой-то игрок бросил «$ roll». Так что ты катишь? Вы катите номер. Может быть, мы могли бы переименовать $roll в $number . Это сделало бы подпись нашего метода довольно естественной: roll($number) .
Но как насчет линии 67? Имеет ли условное выражение значение в контексте функции, если мы переименуем $roll в $number ?
|
1
|
if ($this->isOdd($number)) { … }
|
Мне это не нравится, если я смотрю только на этот кусок кода, я не понимаю, что означает $number . Определение метода — пять строк выше. Возможно, я забыл это, или, возможно, я даже не читал это. Мы также находимся на третьем уровне отступов, наш первоначальный контекст уже сильно изменился. Может быть, требуется более описательное имя. Что насчет $rolledNumber ? Это объясняет тот факт, что это число, и также будет держать его источник в своем названии. Мы знаем, что это число, выбранное игроком. Мы знаем, что это может быть от одного до шести. Это важно? Возможно, в конце концов, мы пытаемся понять унаследованную систему.
Теперь, когда мы решили проблемы с именами параметров и поняли следующие две строки, которые просто выводят текст, мы можем перейти к анализу нашего первого оператора if. Непосредственно перед ним есть также присвоение переменной, но нас это пока не волнует.
Первая часть оператора if довольно большая. Точнее 20 строк, от 66 до 86. Это довольно много информации, которую нужно принять. Может быть, «другая» часть короче. Мы могли бы прокрутить вниз и посмотреть, достаточно ли это легко понять. Эта часть составляет всего около 10-12 строк. И половина из них — строки, выводящие вещи, или пустые, поэтому мы можем заметить, что там не так уж много логики. Может быть, это заслуживает анализа.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
|
} else {
$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $rolledNumber;
if ($this->playerShouldStartANewLap()) {
$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] — $boardSize;
}
echoln($this->players[$this->currentPlayer]
.
.
echoln(«The category is » . $this->currentCategory());
$this->askQuestion();
}
|
Первая строка в if кажется, позиционирует текущего игрока на новое место на доске. Он продвигает игрока по выпавшему номеру. Это типично для настольных игр и звучит довольно логично.
Тогда у нас снова есть условие, но это просто, if проверить, начинает ли игрок новый круг. Если это так, мы помещаем текущего игрока в соответствующую позицию. Помните ли вы, когда мы упростили оператор if , извлекли метод и вызвали его playerShouldStartANewLap() ? Это было, вероятно, более двух месяцев назад. Как полезен этот маленький шаг, чтобы понять эту логику!
Наконец, некоторые вещи отображаются, и вопрос задается в последней строке кода.
Вау. Я только что понял, что мы могли бы объяснить, что здесь происходит, одним предложением: «Игрок помещается на следующую позицию на основе выпавшего номера, информация сообщается пользователю, и мы задаем вопрос» . Когда я могу это сделать, у меня возникает желание быстро создать метод для каждой части, которую я только что определил. У меня в голове уже три простых метода. Хотя я мог полагаться исключительно на свои возможности среды IDE для извлечения методов, я наверняка чувствовал бы себя намного более комфортно, имея несколько тестов для этой части кода. Можем ли мы каким-то образом подготовить игровой объект только с нужными игроками в нужных условиях, чтобы сработала вторая часть оператора if ?
Вторая часть тестирования roll()
Одной из трудностей при тестировании унаследованного кода является приведение SUT (тестируемой системы) в надлежащее состояние, чтобы мы могли проверить интересующее нас состояние. Мы уже знаем, что инициализация класса Game проста. Никакие параметры конструктора не требуются. Затем, если мы посмотрим на список переменных класса. Они просто определены с помощью ключевого слова var . В PHP это означает, что они считаются открытыми переменными. И мы уже использовали currentPlayer в наших предыдущих тестах, поэтому мы можем быть уверены, что можем получить доступ к переменным извне объекта.
На этом этапе мне нравится начинать писать тест. Не полный тест, просто достаточно, чтобы я мог понять, как выполнять SUT.
|
1
2
3
4
5
6
7
|
function testSecondPartOfRollLogic() {
$this->game->currentPlayer = 0;
$this->game->players[$this->game->currentPlayer] = ‘John’;
$this->game->inPenaltyBox[$this->game->currentPlayer] = false;
$this->game->roll(1);
}
|
Название теста пока не очень конкретное. Мы выяснили, что в else части оператора if происходят три вещи, но не совсем понятно, как мы можем определить это всего лишь двумя-тремя словами. Итак, на данный момент мы можем использовать что-то для описания нашего целевого фрагмента кода, который мы хотим использовать. У нас будет время для рефакторинга имени позже, если потребуется.
Затем мы устанавливаем переменные, требуемые кодом. По сути, я скопировал переменные и добавил game после каждого $this-> . Затем я позвонил roll() с номером. Число не имеет значения на данный момент, я просто выбрал номер один, произвольно.
Хотя этот код не имеет утверждений, мы можем выяснить, какая часть кода выполняется, посмотрев на вывод.
|
1
2
3
4
5
|
John is the current player
They have rolled a 1
John’s new location is 1
The category is Science
Science Question 0
|
Мы можем заметить, что «Джон» является нашим текущим игроком, точно так же, как мы определили его в нашем тесте несколькими строками выше, тогда мы можем определить ключевые строки, которые присутствуют только в else части оператора if : «new location is» ,
Таким образом, результат помог нам убедиться, что мы находимся там, где мы должны быть. Следующий шаг — выяснить, что мы должны проверить в нашем тесте.
Мы могли бы проверить следующую позицию игрока, когда он не должен начинать новый круг.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
|
function testSecondPartOfRollLogic() {
$currentPlace = 2;
$rolledNumber = 1;
$this->game->currentPlayer = 0;
$this->game->players[$this->game->currentPlayer] = ‘John’;
$this->game->inPenaltyBox[$this->game->currentPlayer] = false;
$this->game->places[$this->game->currentPlayer] = $currentPlace;
$this->game->roll($rolledNumber);
$this->assertEquals(‘3’, $this->game->places[$this->game->currentPlayer], ‘Player was expected at position 3’);
}
|
OK. Мы написали довольно много новых строк там. Прежде всего, мы определили текущее место и свернули числовые переменные. Затем мы устанавливаем место текущего игрока на доске в положение, указанное выше. После броска мы можем убедиться, что новая позиция игрока, то есть когда игроку не нужно начинать новый круг, является суммой двух чисел.
Поскольку наши тесты проходят, пришло время провести небольшой рефакторинг. Мы пока мало что можем сделать с рабочим кодом, но наш тест требует немного любви.
|
01
02
03
04
05
06
07
08
09
10
11
|
function testSecondPartOfRollLogic() {
$currentPlace = 2;
$rolledNumber = 1;
$this->setAPlayerThatIsNotInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$this->game->roll($rolledNumber);
$this->assertEquals(‘3’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
}
|
Ничего особенного на этот раз, только несколько методов извлечения, чтобы скрыть уродливые вызовы параметров, которые спамили наш класс. Это легче понять, и если нам нужно изменить способ установки или считывания необходимой информации из Game , нам не нужно будет изменять метод тестирования. Мы изменим только приватный метод.
Следующий вопрос: «Что еще мы можем проверить из этого производственного кода?» Мы не хотим вводить, if игроку нужно начать новый круг. Это будет тема другого теста. Два echoln() выводятся на стандартный вывод. Мы мало что можем проверить по этому поводу. Мы могли бы захватить вывод и проверить его, но это презентация. Мы уже можем чувствовать, что здесь присутствует уровень представления, встроенный в бизнес-логику, но мы не можем ясно увидеть, как его безопасно извлечь. Итак, пока, пусть это будет там, не проверено. Затем происходит вызов askQuestion() . Мы должны проверить, что делает этот метод? И если мы сможем это как-то проверить.
askQuestion() проверяет текущую категорию и затем выводит строку пользователю с вопросом. Текущая категория определяется методом currentCategory() , который просто проверяет текущую позицию, и, если она соответствует определенному номеру, выбирается категория. Число три, которое мы использовали в нашем тесте, соответствует категории «Рок». askQuestion() выводит только на экран. Еще одна презентация, которую мы пока не хотим тестировать. Но currentCategory() возвращает строку, строку, которая необходима для askQuestion() . Может быть, мы могли бы вызвать currentCategory() и убедиться, что верная категория возвращена?
|
01
02
03
04
05
06
07
08
09
10
11
12
|
function testSecondPartOfRollLogic() {
$currentPlace = 2;
$rolledNumber = 1;
$this->setAPlayerThatIsNotInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$this->game->roll($rolledNumber);
$this->assertEquals(‘3’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
$this->assertEquals(‘Rock’, $this->game->currentCategory());
}
|
Последняя строка, которую мы добавили, делает именно это. Похоже, нам удалось протестировать всю функциональность нашего целевого фрагмента кода. Теперь мы можем начать рефакторинг производственного кода.
Но ждать! А как насчет случая, когда нам нужно начать новый круг? Разве мы не должны это проверять, прежде чем касаться производственного кода? Я думаю, что сейчас стоит продолжить тестирование и реорганизовать производственный код только тогда, когда мы уверены, что ничего не сломаем.
|
01
02
03
04
05
06
07
08
09
10
11
12
|
function testAPlayerWillStartANewLapWhenNeeded() {
$currentPlace = 11;
$rolledNumber = 2;
$this->setAPlayerThatIsNotInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$this->game->roll($rolledNumber);
$this->assertEquals(‘1’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
$this->assertEquals(‘Rock’, $this->game->currentCategory());
}
|
Мы скопировали и вставили предыдущий тест, соответственно переименовали его и указали разные места и свернутый номер Мы знаем, что размер доски составляет 12 мест. Мы бросили два с места 11, поэтому мы оказались на первой позиции. Нумерация плат начинается с нулевой позиции.
Но наше второе утверждение неверно. Категория «Наука». Этот тест выявил пару проблем с нашим подходом: 1) Нам нужно переименовать наш первый тест, и 2) Нам нужно протестировать категорию в другом тесте. Так что время рефакторинга снова.
|
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
35
|
function testAPlayersNextPositionIsCorrectlyDeterminedWhenNoNewLapIsInvolved() {
// … //
$this->assertEquals(‘3’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
}
function testAPlayerWillStartANewLapWhenNeeded() {
// … //
$this->assertEquals(‘1’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
}
function testRockCategoryCanBeDetermined() {
$currentPlaces = [3];
foreach($currentPlaces as $currentPlace) {
$this->setAPlayerThatIsNotInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$foundCategory = $this->game->currentCategory();
$this->assertEquals(‘Rock’, $foundCategory,
‘Expected rock category for position ‘ .
‘ but got ‘ .
}
}
function testScienceCategoryCanBeDetermined() {
$currentPlaces = [1];
foreach($currentPlaces as $currentPlace) {
$this->setAPlayerThatIsNotInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$foundCategory = $this->game->currentCategory();
$this->assertEquals(‘Science’, $foundCategory,
‘Expected rock category for position ‘ .
‘ but got ‘ .
}
}
|
Мы переименовали первый тест, чтобы отразить точную вещь, которая проверена. В обоих тестах мы убрали проверку категории. Мы знаем, что у нас есть две разные категории и две позиции. Основываясь на наших знаниях и структуре метода currentCategory() , мы можем сделать вывод, что для различных категорий будет несколько мест. Сначала мы определяем места в массивах, а затем ожидаем двух разных значений для двух категорий.
На данный момент наша цель — не тестировать метод currentCategory() . Мы могли бы остановить наш текущий процесс и написать тесты для всех комбинаций мест и категорий. Но я пока не хочу этого делать. Теперь мы должны сосредоточиться на нашем методе roll и небольшом фрагменте кода. Мы все еще можем удалить дублирование между двумя тестами и извлечь проверку в закрытый метод. Это поможет нам в будущем при написании тестов currentCategory() .
|
01
02
03
04
05
06
07
08
09
10
11
|
function testRockCategoryCanBeDetermined() {
$currentPlaces = [3];
$expectedCategory = ‘Rock’;
$this->assertCorrectCategoryForGivenPlaces($expectedCategory, $currentPlaces);
}
function testScienceCategoryCanBeDetermined() {
$currentPlaces = [1];
$expectedCategory = ‘Science’;
$this->assertCorrectCategoryForGivenPlaces($expectedCategory, $currentPlaces);
}
|
Рефакторинг второй части roll()
Теперь, когда все наши тесты прекрасно написаны и проходят, изменение исходного кода является следующим логическим шагом. Оператор if с логикой перемещения игрока должен идти первым.
|
01
02
03
04
05
06
07
08
09
10
|
} else {
$this->movePlayer($boardSize, $rolledNumber);
echoln($this->players[$this->currentPlayer]
.
.
echoln(«The category is » . $this->currentCategory());
$this->askQuestion();
}
|
Похоже, нашему методу понадобятся как минимум два параметра. Размер доски и выпавший номер. Остальная часть используемой информации взята из переменных класса, поэтому их не нужно передавать как параметры. Тем не менее, размер доски также выглядит как значение, которое принадлежит методу, а не классу игры. Позже мы увидим, сможем ли мы переместить это в метод.
|
1
2
3
4
5
6
|
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
}
|
Далее, строки, отображающие вещи для пользователя, должны перейти в их собственные небольшие специализированные методы. Мы могли бы поместить все элементы отображения в один метод, но было бы сложно назвать его. Желательно иметь более именованные и более мелкие методы.
|
1
2
3
4
5
6
7
|
private function displayPlayersNewLocation() {
echoln($this->players[$this->currentPlayer] . «‘s new location is » . $this->places[$this->currentPlayer]);
}
private function displayCurrentCategory() {
echoln(«The category is » . $this->currentCategory());
}
|
С этим мы закончили с этой частью кода. Но как насчет остального метода roll() ?
Два программиста, два разных потока
До сих пор этот урок был сосредоточен на небольших кусочках кода. Наш уровень масштабирования был очень близок к коду. Мы делали много вещей, думая о восьми-десяти строках кода или меньше. Мы сосредоточились на небольших вещах, таких как имена переменных или перемещение одной или двух строк кода из одного места в другое.
Мы в Syneto много занимаемся парным программированием. По сути, каждый раз, когда нужно выполнить хотя бы умеренно сложную задачу, мы делаем это в паре. А рефакторинг — довольно сложная задача, поэтому большую часть времени на код смотрят две пары глаз. Это позволяет нам понимать и думать о том, что мы делаем, глядя на код с двух разных уровней масштабирования. В то время как один программист вносит незначительные изменения и концентрируется на деталях на уровне символов или строк, другой может видеть код на расстоянии.
Хотя формат этих веб-страниц не допускает большого горизонтального пространства, в действительности любой программист должен иметь по крайней мере 25-дюймовый дисплей с приличным разрешением, которое позволяет просматривать код с более высоким уровнем масштабирования. Вот как я см. метод roll() на моем 27-дюймовом дисплее.
На этом уровне человек может наблюдать форму, отступ и облегчение.
Этот человек может думать о сложности, дизайне кода и возможных методах, которые можно извлечь. Этот человек может оценить сложность логики , в то время как другой человек, делающий небольшие изменения, может иметь дело со сложностью кода . Более высокое представление также может достаточно эффективно выделить дублирование.
Разве это не круто? Вы видели огромное дублирование перед этим изображением? Удалось ли вам сконцентрироваться как на мельчайших деталях, так и на высоком уровне обзора? Может быть, вы были. У некоторых людей есть естественный талант понимать код, но большинство из нас не могут эффективно сосредоточиться на более чем одном уровне.
И лучшая часть этого? Вы можете сделать это в одиночку! То есть, если у вас нет возможности соединиться с другим программистом, вы все равно можете увеличивать и уменьшать масштаб. Но вы должны делать это последовательно, чтобы быть эффективными. Примерно так мы и сделали в начале. Мы посмотрели на метод с более высокого уровня, мы определили небольшой фрагмент кода, на который мы могли бы атаковать, и мы увеличили масштаб. Смена фокуса эффективно изменила наше мышление и наш образ мышления. Мы оставались в этом увеличенном состоянии, не думая об остальной части кода, пока не закончили рефакторинг. Теперь мы можем снова уменьшить масштаб, изменить наше мышление и продолжить.
Рефакторинг первой части roll()
Некоторые люди легко меняют свои взгляды и взгляды менее чем за минуту. Другим нужно больше времени, чтобы «забыть» детали и уменьшить масштаб, или наоборот, чтобы перестать думать о форме и начать читать код по одному символу за раз. Если вам нужно десять или 15 минут, это не так уж редко, как вы думаете. Постарайтесь организовать свою работу таким образом, чтобы вы могли сделать свой следующий десятиминутный перерыв между перерывами.
И снова, изменение мышления, нам нужно увеличить первую часть оператора if , когда пользователь находится в штрафной.
Этот код начинается с другого оператора if() проверяющего, является ли выпавший номер нечетным. Если это так, он делает что-то сложное. Если это четное число, оно делает что-то намного проще. Это просто держит игрока в штрафной.
|
1
2
3
4
|
} else {
echoln($this->players[$this->currentPlayer] . » is not getting out of the penalty box»);
$this->isGettingOutOfPenaltyBox = false;
}
|
Это должно быть легко проверить, и это также заставит нас определить способы настройки нашего SUT.
|
1
2
3
4
5
6
7
8
|
function testAPlayerWhoIsPenalizedAndRollsAnEvenNumberWillStayInThePenaltyBox() {
$rolledNumber = 2;
$this->setAPlayerThatIsInThePenaltyBox();
$this->game->roll($rolledNumber);
$this->assertFalse($this->game->isGettingOutOfPenaltyBox);
}
|
Да. Это было легко. Хороший метод испытаний длиной четыре строки. И метод setAPlayerThatIsInThePenaltyBox() очень похож на своего аналога. Единственным отличием является состояние штрафной площадки.
Теперь мы можем начать строить тест или тесты для первой части if, когда бросаемое число нечетное.
|
1
2
3
4
5
6
7
8
|
function testAPlayerWhoIsPenalizedAndRollsAnOddNumberWillGetOutOfThePenaltyBox() {
$rolledNumber = 1;
$this->setAPlayerThatIsInThePenaltyBox();
$this->game->roll($rolledNumber);
$this->assertTrue($this->game->isGettingOutOfPenaltyBox);
}
|
Это многообещающее начало. Первая строка: проверено. Остальные будут похожи на тесты для else части первого уровня if .
Пары должны быть парами до конца
Одна из проблем, с которой я и мои коллеги из Syneto столкнулись при выполнении парного программирования или рефакторинга, заключалась в том, что, когда задача становится понятной и приближается к завершению, один из программистов испытывает желание уйти.
Если вы тот, кто концентрируется на написании тестов и перемещении небольших фрагментов кода, ваша пара может испытать искушение думать, что его роль окончена. Он видел проблемы высокого уровня, сообщал о них вам, теперь ваша очередь исправлять код. Когда он попытается уйти, останови его. Скажите ему, что все задачи начались, поскольку пара должна быть закончена как пара. Скажите ему, чтобы он помог вам подумать о тестах, названиях, структурах, чтобы вы могли сосредоточиться на процедурах и шагах, необходимых для техник рефакторинга, которые необходимо использовать, на шагах, которые мы изучили в предыдущих уроках.
Например, он может подумать об именах тестов, в то время как вы заняты копированием / вставкой и изменением предыдущих тестов, чтобы принудительно ввести запись в этой части if.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
|
function testPlayerGettingOutOfPenaltyNextPositionWithoutNewLap() {
$currentPlace = 2;
$numberRequiredToGetOutOfPenaltyBox = 1;
$this->setAPlayerThatIsInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$this->game->roll($numberRequiredToGetOutOfPenaltyBox);
$this->assertEquals(‘3’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
}
function testPlayerGettingOutOfPenaltyNextPositionWithNewLap() {
$currentPlace = 11;
$numberRequiredToGetOutOfPenaltyBox = 3;
$this->setAPlayerThatIsInThePenaltyBox();
$this->setCurrentPlayersPosition($currentPlace);
$this->game->roll($numberRequiredToGetOutOfPenaltyBox);
$this->assertEquals(‘2’, $this->getCurrentPlayersPosition(), ‘Player was expected at position 3’);
}
|
С другой стороны, если ваша пара решит, что, обнаружив все дублирование и классные проблемы высокого уровня, он сможет написать тесты и выполнить рефакторинг кода, вы можете почувствовать, что вам больше нечего делать. Но вы не правы. Вы можете остаться и помочь ему с именами, дублированием низкого уровня и другими мелочами. Вы также можете помочь ему, когда он спотыкается или пропускает какие-то маленькие шаги или когда он застревает на неудачных тестах из-за глупой маленькой опечатки.
Вот как правильно будут называться тесты, например testPlayerGettingOutOfPenaltyNextPositionWithNewLap() и переменные будут выражать то, что они представляют для текущего теста, а не то, что они делали для предыдущего теста, который вы скопировали: $numberRequiredToGetOutOfPenaltyBox .
|
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
|
function roll($rolledNumber) {
echoln($this->players[$this->currentPlayer] . » is the current player»);
echoln(«They have rolled a » . $rolledNumber);
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
$this->movePlayer($boardSize,$rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
} else {
echoln($this->players[$this->currentPlayer] . » is not getting out of the penalty box»);
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
}
}
|
Разве это не лучше, чем раньше? Все юнит-тесты проходят. Но я чувствую, что мы переместили много кода, отвечающего за презентацию. Может быть, мы должны запустить наш золотой мастер-тест?
Делая шаг назад
|
1
2
3
4
5
|
PHPUnit_Framework_ExpectationFailedException : Failed asserting that false is true.
Time: 18.93 seconds, Memory: 112.50Mb
FAILURES!
Tests: 20, Assertions: 33, Failures: 1, Skipped: 1.
|
И это не удается. Прошло довольно много времени с тех пор, как мы запускали наши золотые мастер-тесты, но все наши модификации были локализованы в методе roll() . Так что худшее, что может случиться, это то, что мы отменили наши изменения.
Давайте начнем с небольшого шага назад. Я подозреваю, что там, где мы видели дублирование в выводе, была небольшая разница. Может быть, письмо или пробел мы не наблюдали. Мы могли бы вернуть вывод в первой части roll() и посмотреть, работает ли он.
|
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
|
function roll($rolledNumber) {
echoln($this->players[$this->currentPlayer] . » is the current player»);
echoln(«They have rolled a » . $rolledNumber);
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
$this->movePlayer($boardSize,$rolledNumber);
echoln($this->players[$this->currentPlayer]
.
.
echoln(«The category is » . $this->currentCategory());
$this->askQuestion();
} else {
echoln($this->players[$this->currentPlayer] . » is not getting out of the penalty box»);
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
}
}
|
Это все еще не удается. Поскольку наше первое подозрение было неверным, мы можем захотеть сделать больший шаг назад. Наш золотой мастер прошел до того, как мы начали нашу работу? Может быть, в следующий раз мы должны начать с этого. Теперь нам нужно принять наши изменения, поместить их в безопасное место и отменить весь код, чтобы проверить нашу гипотезу.
Возврат к исходному состоянию roll() заставляет золотого мастера пройти. Хорошо знать. Итак, мы сломали это. Но когда? Где?
Теперь, когда наш код возвращен к исходному, мы можем наблюдать за выводом, помещать его в текстовый файл и сравнивать с переделанным.
Как мы можем сразу заметить, мы пропустили некоторые строки в реорганизованной версии. Строки, говорящие нам о том, что игрок выходит из штрафной, отсутствуют. Хм …
Давайте снова посмотрим на код, с которого мы начали. Ага!!! Вот оно!
|
1
2
3
4
5
|
echoln($this->players[$this->currentPlayer] . » is getting out of the penalty box»);
$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] + $roll;
if ($this->playerShouldStartANewLap()) {
$this->places[$this->currentPlayer] = $this->places[$this->currentPlayer] — $boardSize;
}
|
echoln() застрял на вершине движущейся логики. Честная, простая ошибка. Мы этого не наблюдали, просто взяли весь этот блок кода и заменили его вызовом метода.
|
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
|
function roll($rolledNumber) {
echoln($this->players[$this->currentPlayer] . » is the current player»);
echoln(«They have rolled a » . $rolledNumber);
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
echoln($this->players[$this->currentPlayer] . » is getting out of the penalty box»);
$this->movePlayer($boardSize,$rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
} else {
echoln($this->players[$this->currentPlayer] . » is not getting out of the penalty box»);
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
}
}
|
Это делает все тесты успешными. Слава Богу, у меня была пара, чтобы помочь мне понять это. Несмотря на то, что мой был плюшевым мишкой, когда я пишу эти статьи в одиночку, много раз это помогает просто рассказать кому-то о вашей проблеме. Это заставит ваш разум переиграть все мысли и переделать процесс. Более того, это заставляет вас осознать глупые ошибки и наблюдать за вещами, которые вы иначе упустили бы
Добавляем последний штрих
Прежде чем мы завершим этот урок, мы должны убедиться, что мы оставляем наш метод roll() в лучшей форме, какую только можем. Во-первых, все echoln() могут переходить в закрытые методы.
|
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
|
function roll($rolledNumber) {
$this->displayCurrentPlayer();
$this->displayRolledNumber($rolledNumber);
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
$this->displayPlayerGettingOutOfPenaltyBox();
$this->movePlayer($boardSize,$rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
} else {
$this->displayPlayerStaysInPenaltyBox();
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayPlayersNewLocation();
$this->displayCurrentCategory();
$this->askQuestion();
}
}
|
Выше это шаг в правильном направлении, но моя пара говорит, что мы можем добиться большего.
Мы можем сгруппировать последовательные функции отображения в другие функции отображения.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
|
function roll($rolledNumber) {
$this->displayStatusAfterRoll($rolledNumber);
$boardSize = 12;
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
$this->movePlayer($boardSize,$rolledNumber);
$this->displayStatusAfterPlayerGettingOutOfPenaltyBox();
$this->askQuestion();
} else {
$this->displayPlayerStaysInPenaltyBox();
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($boardSize, $rolledNumber);
$this->displayStatusAfterNonPenalizedPlayerMove();
$this->askQuestion();
}
}
|
Разве это не лучше? Только один вызов дисплея с каждым путем, по которому может идти наш метод.
Вы помните $boardSize ? Можем ли мы переместить его внутрь movePlayer() сейчас? Да мы можем. Итак, давайте сделаем это.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
|
function roll($rolledNumber) {
$this->displayStatusAfterRoll($rolledNumber);
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->isGettingOutOfPenaltyBox = true;
$this->movePlayer($rolledNumber);
$this->displayStatusAfterPlayerGettingOutOfPenaltyBox();
$this->askQuestion();
} else {
$this->displayPlayerStaysInPenaltyBox();
$this->isGettingOutOfPenaltyBox = false;
}
} else {
$this->movePlayer($rolledNumber);
$this->displayStatusAfterNonPenalizedPlayerMove();
$this->askQuestion();
}
}
|
Наш код становится довольно минимальным. Но все же этот метод имеет длину 18 строк. Это много. Вы помните учение Роберта К. Мартина или «Магическое число семь плюс минус два»? Наши методы были бы лучше, если бы они содержали всего около четырех строк кода.
Первым шагом в этом направлении является сокращение его до одного вызова функции для каждого возможного пути.
|
01
02
03
04
05
06
07
08
09
10
11
12
13
|
function roll($rolledNumber) {
$this->displayStatusAfterRoll($rolledNumber);
if ($this->inPenaltyBox[$this->currentPlayer]) {
if ($this->isOdd($rolledNumber)) {
$this->getPlayerOutOfPenaltyBoxAndPlayNextMove($rolledNumber);
} else {
$this->keepPlayerInPenaltyBox();
}
} else {
$this->playNextMove($rolledNumber);
}
}
|
Теперь у нас до 12 строк кода. Но мы можем сделать еще лучше. Самое сокровенное, if можно войти в его собственный метод.
|
1
2
3
4
5
6
7
8
|
function roll($rolledNumber) {
$this->displayStatusAfterRoll($rolledNumber);
if ($this->inPenaltyBox[$this->currentPlayer]) {
$this->playNextMoveForPlayerInPenaltyBox($rolledNumber);
} else {
$this->playNextMove($rolledNumber);
}
}
|
Мы закончили!
После этого мы сократили до семи строк кода в нашем методе. Только пять внутри метода, и только четыре фактически выполняют какую-то логику. Теперь это разумно выглядящий метод, и это тот момент, когда я бы хотел остановиться. Кроме того, это не просто пример. Это «Извлечение до упаду», и так выглядит большинство наших методов в наших проектах в Syneto. Это пример из реальной жизни, в котором вы должны изо дня в день заканчивать всем своим кодом. Здесь мы также остановимся на этом уроке.
Оставайтесь с нами для следующего урока, где мы поговорим о слоях и начнем разделять проблемы.