Рефакторинг унаследованного кода: Часть 3 - Сложные условия

  1. Статьи
  2. Веб-программирование
  3. PHP
0 ∞

Содержание цикла статей "Рефакторинг унаследованного кода":

Часть 1 - Золотой мастер;
Часть 2 - Магические строки и константы;
Часть 3 - Сложные условия;
Часть 4 - Наш первый модульный тест;

Старый код. Плохой код. Сложный код. Запутанный код. Полная ерунда. Два слова - унаследованный код. Это серия статей поможет вам работать с ним.

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

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

Содержание

От прозы к коду

Вот пример «прозы», чтобы немного поднять вам настроение. Во-первых, здесь все намешано в кучу. Просто ужасно:

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

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

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

Упрощение

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

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

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

Теперь давайте соберем все это вместе:

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

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

Сколько раз вы интересовались деталями реализации метода logInfo("Some message");? Скорее всего, только раз, если и когда вы реализовали его. После этого он просто регистрирует сообщение в категории «информация».

Или когда пользователь приобретает один из ваших товаров, вы заботитесь о том, как выставить ему счет? Нет, вы беспокоитесь о том, был ли куплен продукт, списан ли он со склада и выставлен ли счет покупателю (все равно как).

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

Сложные условия

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

Строка 20 файла GameRunner.php выглядит так:

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

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

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

Руководство по условным тегам: финальная часть

Рефакторинг методом извлечения

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

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

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

  • создайте новый метод и назовите его по принципу «После чего он вступает в действие», а не «Как он работает»;
  • скопируйте код из извлекаемого места в метод. Пожалуйста, обратите внимание, что это копия, не удаляйте пока исходный код;
  • просканируйте извлеченный код на наличие любых переменных, которые являются локальными. Они должны быть заданы в качестве параметров метода;
  • проверьте, не используются ли какие-либо временные переменные внутри извлекаемого метода. Если это так, объявите их внутри метода и задайте дополнительный параметр;
  • передайте в новый метод переменные в качестве параметров;
  • замените извлекаемый код вызовом нового метода;
  • запустите тесты.

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

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

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

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

Рефакторинг методом извлечения

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

// ... //
$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);

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

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!

Давайте посмотрим на тесты. В них содержится метод GameRunner.php, который выполняет действие require() для файла GameRunner.php. Он вызывается, по крайней мере, дважды. В этом и заключается причина ошибки.

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

private function generateOutput($seed) {
    ob_start();
    srand($seed);
    require __DIR__ . '/../trivia/php/GameRunner.php';
    $output = ob_get_contents();
    ob_end_clean();
    return $output;
}
Изучаем теги шаблонов: часть шестая

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

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

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

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

include_once __DIR__ . '/Game.php';

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-файл, включающий или подтягивающий элемент запуска, а затем непосредственно вызывающий в нем метод запуска. Не то, чтобы это было очень существенное изменение, но об этом нужно помнить, особенно если существующий код будут использовать третьи лица.

В то же время мы можем сразу же включить и такой файл в наш тест:

require __DIR__ . '/../trivia/php/GameRunner.php';

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

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, проигрывает викторину, после чего генерируется новый файл золотого мастера. Что, если мы введем еще один файл? Мы сделаем так, что файл GameRunner.php будет непосредственно запускать викторину через вызов run() и ничего больше.

Если в нем нет никакой логики, которая бы могла изменить свое поведение, и для него не нужны никакие тесты, тогда мы можем перенести наш текущий код в другой файл?

Структура папок, файлы и назначение имен - 2

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

Нет логики - значит, не нужны тесты:

require_once __DIR__ . '/RunnerFunctions.php';
run();

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

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

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

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

Полный SEO-аудит. Часть 1 из 3: Технические аспекты

Вычищаем файл RunnerFunctions

Если мы посмотрим на файл RunnerFunctions.php, то увидим, что в нем появилась определенная неразбериха после изменений, которые мы произвели.

Мы определили:

$minAnswerId = 0;
$maxAnswerId = 9;
$wrongAnswerId = 7;

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

function isCurrentAnswerWrong() {
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) == $wrongAnswerId;
}

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

Отрицательные условия

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

Было бы гораздо легче понять метод, который проверяет правильность ответа, и если он не является таковым, отбрасывает его, когда это необходимо:

function isCurrentAnswerCorrect() {
    $minAnswerId = 0;
    $maxAnswerId = 9;
    $wrongAnswerId = 7;
    return rand($minAnswerId, $maxAnswerId) != $wrongAnswerId;
}

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

if (!isCurrentAnswerCorrect()) {
    $notAWinner = $aGame->wrongAnswer();
} else {
    $notAWinner = $aGame->wasCorrectlyAnswered();
}

Это делает условие более понятным для нас. Использование ! в операторе if(), на самом деле говорит нам многое. Символ размещается отдельно и подчеркивается тот факт, что что-то отрицается.

Но можем ли мы как-то изменить это, чтобы обойтись вообще без отрицания? Да, можем:

if (isCurrentAnswerCorrect()) {
    $notAWinner = $aGame->wasCorrectlyAnswered();
} else {
    $notAWinner = $aGame->wrongAnswer();
}

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

Условия в Game.php

Файл RunnerFunctions.php мы упростили по максимуму. Давайте теперь возьмемся за файл Game.php. Есть несколько способов, с помощью которых вы можете осуществить в нем поиск условий.

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

Второй очевидный способ - это просто осуществить поиск операторов "if" или "if (". Если вы отформатировали код с помощью встроенной функции IDE, вы можете быть уверены, что все условные операторы имеют одинаковый специфический формат.

В моем случае, все ключевые слова "if" пишутся через пробел со скобкой. Кроме того, если вы используете встроенный поиск, все найденные результаты будут выделены ярким цветом, в моем случае желтым:

Условия в Game.php

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

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

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

if ($roll % 2 != 0)

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

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

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

Так уже намного лучше, потому что теперь не нужно отвлекать внимание от основной проблемы:

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

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

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

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

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

Абсолютно нет! Итак, давайте переименуем этот метод так, чтобы if представлял то, какие действия происходят в этом операторе, а не то, что мы сверяем с условием:

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

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

Тем не менее, даже опытный программист вынужден изменять названия методов, по крайней мере, по три-пять раз, прежде чем найдет правильное имя. Так что не бойтесь часто нажимать Ctrl + R и переименовывать элементы.

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

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

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

И не забывайте запускать тесты до и после, даже когда вы осуществляете рефакторинг с помощью магии IDE. Что приводит нас к следующему наблюдению. Магия, иногда, не срабатывает, посмотрите на строку 65:

$lastPositionOnTheBoard = 11;

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

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

И не забудьте вызвать метод без параметров в операторе if:

if ($this->playerShouldStartANewLap())

С оператором if в методе askQuestion(), кажется, все в порядке. Также как и с тем, что содержится в currentCategory():

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

Этот случай немного сложнее, но в принципе он и так отвечает нашим критериям и называется достаточно понятно:

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

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

if ($this->shouldResetCurrentPlayer())

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

Тесты проходят, а все операторы if были сделаны нами максимально упрощенными.

Заключение

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

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

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

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

РедакцияПеревод статьи «Refactoring Legacy Code: Part 3 - Compile Conditionals»