• Главная»
  • Уроки»
  • PHP»
  • Рефакторинг унаследованного кода: часть 3 - сложные условия

Этот урок связан с проектом Рефакторинг унаследованного кода

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

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

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

Переходя от прозы к коду

Вот вам пример из прозы, который объяснит мою точку зрения. Для начала рассмотрим предложение “всё в одном”. Некрасивое.

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

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

Упрощение

Давайте попробуем его немного упростить. Всё, начиная с первого слова, до слова “тогда” является условием. Да, оно сложное, но мы можем сократить его до “Если условия окружающей среды представляют угрозу...”, тогда нужно что-то сделать. Сложное выражение говорит о том, что мы должны оповестить кого-то, кто удовлетворяет большому количеству условий: “тогда необходимо оповестить персонал поддержки третьего уровня”. И, наконец, описывается весь процесс, от момент пробуждения до момента, когда ситуация будет исправлена техником: “и ожидается, что состояние окружающей среды будет восстановлено до нормальных параметров”. Давайте объединим всё это вместе:

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

Сейчас это предложение занимает около 20% от первоначального объёма. Нам не известны все детали, но в большинстве случаев нам этого и не надо. И это также относится и к исходному коду. Как часто вы анализировали детали реализации метода logInfo("Some message");? Возможно, всего однажды, тогда, когда (и если) вы его реализовали. Он всего-лишь записывает информацию в категорию “info”. А когда пользователь покупает ваш продукт, вы следите за каждым шагом расчёта? Нет. Всё, о чём вам нужно позаботиться - если продукт был куплен, вычесть необходимое количество со склада, и передать его покупателю. Примеры можно приводить бесконечно. Все они говорят о том, как писать правильное программное обеспечение.

Сложные выражения

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

Строка 12 файла GameRunner.php выглядит следующим образом:

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

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

Оно не очень сложное, но мы можем его упростить. Как насчёт этого: Если выбран неверный вариант ответа, тогда…. Лучше, вам не кажется?

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

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

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

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

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

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

Просто выделите нужный участок кода, и вызовите контекстное меню:

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 она определена только однажды!

Взглянем на тесты. У нас есть метод generateOutput(), который подключает файл GameRunner.php посредством require(). И он вызывается минимум дважды. Вот и источник ошибки.

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

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);
}

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

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

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(), и не делая ничего более. В нём не останется логики, где что-то могло бы пойти не так, и для него больше не нужны тесты, а текущий код можно перенести в другой файл.

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

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

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

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

Подчищаем RunnerFunction.php

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

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

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

Звучит осмысленно. Можно вынести этот фрагмент в метод, но найдём ли мы ему правильное имя, чтобы условие стало понятнее?

if ($roll % 2 != 0)

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

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

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

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

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

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

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

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

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

Выражение 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 были упрощены.

Заключение

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

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

Данный урок подготовлен для вас командой сайта ruseller.com
Источник урока: http://code.tutsplus.com/tutorials/refactoring-legacy-code-part-3-complex-conditionals--cms-20944
Перевел: Станислав Протасевич
Урок создан: 11 Декабря 2014
Просмотров: 6317
Правила перепечатки


5 последних уроков рубрики "PHP"

  • Фильтрация данных с помощью zend-filter

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

  • Контекстное экранирование с помощью zend-escaper

    Обеспечение безопасности веб-сайта — это не только защита от SQL инъекций, но и протекция от межсайтового скриптинга (XSS), межсайтовой подделки запросов (CSRF) и от других видов атак. В частности, вам нужно очень осторожно подходить к формированию HTML, CSS и JavaScript кода.

  • Подключение Zend модулей к Expressive

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

  • Совет: отправка информации в Google Analytics через API

    Предположим, что вам необходимо отправить какую-то информацию в Google Analytics из серверного скрипта. Как это сделать. Ответ в этой заметке.

  • Подборка PHP песочниц

    Подборка из нескольких видов PHP песочниц. На некоторых вы в режиме online сможете потестить свой код, но есть так же решения, которые можно внедрить на свой сайт.

^ Наверх ^