Статьи

Рефакторинг устаревшего кода: часть 3 — сложные условия

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

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

Вот «прозаический» пример, чтобы подбодрить вас. Во-первых, предложение «все в одном». Уродливый.

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

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

Давайте найдем способ немного упростить это. Вся его первая часть, вплоть до «тогда», является условием. Да, это сложно, но мы могли бы подвести итог так: если условия окружающей среды представляют риск … … тогда что-то должно быть сделано. Сложное выражение говорит, что мы должны уведомить кого-то, кто удовлетворяет многим условиям: затем уведомить техническую поддержку третьего уровня . Наконец, описывается целый процесс от пробуждения технического специалиста до тех пор, пока все не будет исправлено: и ожидается, что среда будет восстановлена ​​в рамках нормальных параметров. Давайте сложим все это вместе.

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

Теперь это всего лишь около 20% по сравнению с исходным текстом. Мы не знаем деталей, и в значительном большинстве случаев нам все равно. И это очень верно и для исходного кода. Сколько раз вы заботились о деталях реализации logInfo("Some message"); метод? Вероятно, один раз, если и когда вы это реализовали. Затем он просто регистрирует сообщение в категории «информация». Или когда пользователь покупает один из ваших продуктов, вас волнует, как ему выставить счет? Нет. Все, о чем вы хотите заботиться, — это если продукт был куплен, выбросьте его из инвентаря и выставьте счет покупателю . Примеры могут быть бесконечными. Они в основном, как мы пишем правильное программное обеспечение.

В этом разделе мы попытаемся применить философию прозы к нашей викторине. Один шаг за раз. Начиная со сложных условностей. Давайте начнем с простого кода. Просто чтобы согреться.

Строка двадцать файла GameRunner.php так:

1
if (rand($minAnswerId, $maxAnswerId) == $wrongAnswerId)

Как бы это звучало в прозе? Если случайное число между минимальным идентификатором ответа и максимальным идентификатором ответа равно идентификатору неправильного ответа, то …

Это не очень сложно, но мы все еще можем сделать это проще. Что насчет этого? Если выбран неправильный ответ, тогда … Лучше, не так ли?

Нам нужен способ, процедура, техника, чтобы переместить это условное утверждение куда-нибудь еще. Этот пункт назначения может легко быть методом. Или в нашем случае, поскольку мы не находимся здесь внутри класса, функции. Это перемещение поведения в новый метод или функцию называется рефакторингом «Извлечь метод». Ниже приведены шаги, определенные Мартином Фаулером в его превосходной книге «Рефакторинг: улучшение дизайна существующего кода». Если вы не читали эту книгу, вы должны поместить ее в свой список «Для чтения» сейчас. Это одна из самых важных книг для современного программиста.

Для нашего урока я предпринял оригинальные шаги и немного упростил их, чтобы лучше соответствовать нашим потребностям и типу урока.

  1. Создайте новый метод и назовите его после того, что он делает, а не как он это делает.
  2. Скопируйте код из извлеченного места, в метод. Обратите внимание, что это копия , пока не удаляйте исходный код.
  3. Сканирование извлеченного кода для любых локальных переменных. Они должны быть сделаны параметры для метода.
  4. Посмотрите, используются ли какие-либо временные переменные внутри извлеченного метода. Если это так, объявите их внутри и удалите дополнительный параметр.
  5. Передайте в целевой метод переменные в качестве параметров.
  6. Замените извлеченный код вызовом целевого метода.
  7. Запустите ваши тесты.

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

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

Просто выберите нужную часть кода и щелкните ее правой кнопкой мыши .

Среда IDE автоматически поймет, что для запуска нашего кода нам нужны три параметра, и предложит следующее решение.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
// … //
$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;
function isCurrentAnswerWrong($minAnswerId, $maxAnswerId, $wrongAnswerId) {
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}
 
do {
    $dice = rand(0, 5) + 1;
    $aGame->roll($dice);
 
    if (isCurrentAnswerWrong($minAnswerId, $maxAnswerId, $wrongAnswerId)) {
        $notAWinner = $aGame->wrongAnswer();
    } else {
        $notAWinner = $aGame->wasCorrectlyAnswered();
    }
 
} while ($notAWinner);

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

1
2
3
4
5
6
7
Fatal error: Cannot redeclare isCurrentAnswerWrong()
(previously declared in /home/csaba/Personal/Programming/NetTuts
/Refactoring Legacy Code — Part 3: Complex Conditionals and Long Methods
/Source/trivia/php/GameRunner.php:16)
in /home/csaba/Personal/Programming/NetTuts
/Refactoring Legacy Code — Part 3: Complex Conditionals and Long Methods
/Source/trivia/php/GameRunner.php on line 18

В основном это говорит о том, что мы хотим объявить функцию дважды. Но как это может произойти? У нас это есть только один раз в нашем GameRunner.php !

Посмотрите на тесты. Есть метод generateOutput() который выполняет require() в нашем GameRunner.php . Это называется как минимум дважды. Вот источник ошибки.

Теперь у нас есть дилемма. Из-за заполнения генератора случайных чисел нам нужно вызывать этот код с контролируемыми значениями.

1
2
3
4
5
6
7
8
private function generateOutput($seed) {
    ob_start();
    srand($seed);
    require __DIR__ .
    $output = ob_get_contents();
    ob_end_clean();
    return $output;
}

Но нет способа объявить функцию дважды в PHP, поэтому нам нужно другое решение. Мы начинаем чувствовать бремя нашего золотого мастер-тестирования. Выполнение всего этого 20000 раз, каждый раз, когда мы меняем фрагмент кода, может не быть решением в долгосрочной перспективе. Помимо того, что на запуск уходит целая вечность, это заставляет нас изменять наш код в соответствии с тем, как мы его тестируем. Обычно это признак плохих тестов. Код должен измениться и все же пройти тест, но изменения должны иметь причины для изменения, исходящие только из исходного кода.

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

Один из способов решения нашей проблемы — взять весь остальной код в GameRunner.php и поместить его в функцию. Допустим, run()

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
include_once __DIR__ .
 
function isCurrentAnswerWrong($minAnswerId, $maxAnswerId, $wrongAnswerId) {
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}
 
function run() {
    $notAWinner;
 
    $aGame = new Game();
 
    $aGame->add(«Chet»);
    $aGame->add(«Pat»);
    $aGame->add(«Sue»);
 
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    do {
        $dice = rand(0, 5) + 1;
        $aGame->roll($dice);
 
        if (isCurrentAnswerWrong($minAnswerId, $maxAnswerId, $wrongAnswerId)) {
            $notAWinner = $aGame->wrongAnswer();
        } else {
            $notAWinner = $aGame->wasCorrectlyAnswered();
        }
 
    } while ($notAWinner);
}

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

С другой стороны, теперь мы можем просто включить файл в наш тест.

1
require __DIR__ .

А затем вызовите run() внутри метода generateOutput ().

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

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

Наши тестовые вызовы run() в файле GameRunner.php , который включает в себя Game.php , Game.php игру и генерирует новый золотой мастер-файл. Что если мы представим другой файл? Мы заставляем GameRunner.php запускать игру, вызывая run() и ничего больше. Так что, если там нет логики, которая может пойти не так, и тесты не нужны, а затем мы переместим наш текущий код в другой файл?

Теперь это совсем другая история. Теперь наши тесты получают доступ к коду прямо под бегуном. По сути, наши тесты просто бегуны. И, конечно же, в нашем новом GameRunner.php будет только звонок для запуска игры. Это настоящий бегун, он ничего не делает, кроме как вызывает метод run() . Отсутствие логики означает, что тесты не нужны.

1
2
require_once __DIR__ .
run();

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

Мы также попытались назвать наши файлы в соответствии с тем, что делает код внутри них. Одна из них — это просто набор функций для бегуна, функции, которые мы на данный момент считаем принадлежащими друг другу, чтобы удовлетворить потребности бегуна. Станет ли это классом в будущем? Может быть. Возможно, нет. Пока это достаточно хорошо.

Если мы посмотрим на файл RunnerFunctions.php , мы увидим небольшую путаницу.

Мы определяем:

1
2
3
$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;

… внутри метода run() . У них есть одна причина существования и единственное место, где они используются. Почему бы просто не определить их внутри этого метода и вообще избавиться от параметров?

1
2
3
4
5
6
function isCurrentAnswerWrong() {
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}

Хорошо, тесты проходят, и код намного приятнее. Но не достаточно хорошо.

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

1
2
3
4
5
6
function isCurrentAnswerCorrect() {
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) != $wrongAnswerId;
}

Мы использовали рефакторинг Rename Method. Опять же, это довольно сложно, если использовать вручную, но в любой IDE это так же просто, как нажать CTRL + r или выбрать соответствующую опцию в меню. Чтобы наши тесты прошли успешно, нам также необходимо обновить наш условный оператор с отрицанием.

1
2
3
4
5
if (!isCurrentAnswerCorrect()) {
    $notAWinner = $aGame->wrongAnswer();
} else {
    $notAWinner = $aGame->wasCorrectlyAnswered();
}

Это приближает нас на один шаг к нашему пониманию условного. Используя ! в выражении if() , на самом деле помогает. Он выделяется и подчеркивает тот факт, что что-то там на самом деле отрицается. Но можем ли мы изменить это, чтобы полностью избежать отрицания? Да мы можем.

1
2
3
4
5
if (isCurrentAnswerCorrect()) {
    $notAWinner = $aGame->wasCorrectlyAnswered();
} else {
    $notAWinner = $aGame->wrongAnswer();
}

Теперь у нас нет логического отрицания при использовании ! ни лексическое отрицание, называя и возвращая неправильные вещи. Все эти шаги сделали наши условия намного проще для понимания.

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

Второй очевидный способ поиска условных выражений — это просто найти «if» или «if (». Если вы отформатировали код со встроенными функциями вашей IDE, вы можете быть уверены, что все условные операторы имеют та же конкретная форма. В моем случае между «if» и круглыми скобками есть пробел. Кроме того, если вы используете встроенный поиск, найденные результаты будут выделены резким цветом, в моем случае желтым.

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

1
if ($this->inPenaltyBox[$this->currentPlayer])

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

1
if ($roll % 2 != 0)

Бьюсь об заклад, 90% всех программистов могут понять проблему в приведенном выше утверждении if . Мы пытаемся сосредоточиться на том, что делает наш текущий метод. И наш мозг подключен к области проблемы. Мы не хотим «запускать другой поток» для вычисления этого математического выражения, чтобы понять, что оно просто проверяет, является ли число нечетным. Это одно из тех маленьких отвлекающих факторов, которые могут испортить сложный логический вывод. Итак, я говорю, давайте извлекать это.

1
if ($this->isOdd($roll))

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

1
if ($this->places[$this->currentPlayer] > $lastPositionOnTheBoard)

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

1
if ($this->playerReachedEndOfBoard($lastPositionOnTheBoard))

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

1
if ($this->playerShouldStartANewLap($lastPositionOnTheBoard))

Когда вы пытаетесь назвать методы и переменные, всегда думайте о том, что должен делать код, а не о том, какое состояние или условие он представляет. Как только вы получите это право, действия по переименованию в вашем коде значительно упадут. Но, тем не менее, даже опытному программисту необходимо переименовать метод как минимум три-пять раз, прежде чем найти его правильное имя. Так что не бойтесь нажимать CTRL + r и часто переименовывать. Никогда не вносите свои изменения в VCS проекта, если вы не сканировали имена вновь добавленных методов и ваш код не читается как хорошо написанная проза. Переименование в наши дни настолько дешево, что вы можете переименовывать вещи просто для того, чтобы опробовать различные версии и вернуться одним нажатием кнопки.

Оператор if в строке 90 такой же, как и предыдущий. Мы можем просто повторно использовать наш извлеченный метод. Вуаля, дублирование устранено! И не забывайте запускать свои тесты время от времени, даже когда вы проводите рефакторинг, используя магию вашей IDE. Что приводит нас к следующему наблюдению. Магия, иногда, терпит неудачу. Проверьте строку 65.

1
$lastPositionOnTheBoard = 11;

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

1
2
3
4
private function playerShouldStartANewLap() {
    $lastPositionOnTheBoard = 11;
    return $this->places[$this->currentPlayer] > $lastPositionOnTheBoard;
}

И не забудьте вызвать метод без каких-либо параметров в ваших операторах if .

1
if ($this->playerShouldStartANewLap())

askQuestion() if операторы if в askQuestion() в порядке, как и в currentCategory() .

1
if ($this->inPenaltyBox[$this->currentPlayer])

Это немного сложнее, но в области и достаточно выразительно.

1
if ($this->currentPlayer == count($this->players))

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

1
if ($this->shouldResetCurrentPlayer())

Это намного лучше, и мы будем использовать его повторно в строках 172, 189 и 203. Дублирование, я имею в виду тройное дублирование, я имею в виду четырехкратное дублирование, устранено!

Тесты проходят, и все, if заявления были оценены на сложность.

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

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