Рефакторинг унаследованного кода: Часть 1 - Золотой мастер

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

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

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

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

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

Определение унаследованного кода

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

На мой взгляд, унаследованный код - это просто код без тестов. ~ Майкл Физерс

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

Однако это определение имеет несколько иной смысл. Оно очень четко объясняет проблему, так что становится очевидным и ее решение. «Трудно изменить» - это как-то расплывчато. Что мы должны сделать, чтобы его было легко изменить? Мы понятия не имеем!

С другой стороны, «код без тестов» - это очень конкретно. И ответ на наш вопрос очень прост - сделать код тестируемым и протестировать его. Так что давайте начнем.

Получение унаследованного кода

Эта серия статей будет основана на исключительной викторине Джей Би Рейнсбергера, разработанной им для мероприятий Legacy Code Retreat. Она сделана так, чтобы быть похожей на реальный унаследованный код, и в ней можно было применить широкий спектр приемов рефакторинга достаточно серьезного уровня сложности.

Проверка исходного кода

Викторина размещена на GitHub и имеет GPLv3 лицензию, так что вы сами можете в нее легко поиграть. Мы начнем эту серию с проверки официального репозитория. К статье также прилагается код со всеми изменениями, которые мы произведем, так что если вы в какой-то момент запутаетесь, вы можете просмотреть конечный результат:

$ git clone https://github.com/jbrains/trivia.git
Cloning into 'trivia'...
remote: Counting objects: 429, done.
remote: Compressing objects: 100% (262/262), done.
remote: Total 429 (delta 100), reused 419 (delta 93)
Receiving objects: 100% (429/429), 848.33 KiB | 305.00 KiB/s, done.
Resolving deltas: 100% (100/100), done.
Checking connectivity... done.

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

Осмысление кода

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

В нашей папке содержатся два файла:

$ cd php/
$ ls -al
total 20
drwxr-xr-x  2 csaba csaba 4096 Mar 10 21:05 .
drwxr-xr-x 26 csaba csaba 4096 Mar 10 21:05 ..
-rw-r--r--  1 csaba csaba 5568 Mar 10 21:05 Game.php
-rw-r--r--  1 csaba csaba  410 Mar 10 21:05 GameRunner.php

Выглядит так, что код мы можем запустить через файл GameRunner.php:

$ php ./GameRunner.php
Chet was added (Добавлен Чет)
They are player number 1 (Он становится участником номер 1)
Pat was added (Добавлен Пэт)
They are player number 2 (Он становится участником номер 2)
Sue was added (Добавлена Сью)
They are player number 3 (Она становится участницей номер 3)
Chet is the current player (На вопрос отвечает Чет)
They have rolled a 4 (Он выбросил 4)
Chet's new location is 4 (Его текущее положение - 4)
The category is Pop (категория - Поп)
Pop Question 0 (Вопрос категории Поп 0)
Answer was corrent!!!! (Ответ правильный!!!!)
Chet now has 1 Gold Coins. (Чет зарабатывает 1 золотую монету)
Pat is the current player (Теперь на вопрос отвечает Пэт)
They have rolled a 2 (Он выбросил 2)
Pat's new location is 2 (Его текущее положение - 2)
The category is Sports (категория - Спорт)
Sports Question 0 (Вопрос категории Спорт 0)
Answer was corrent!!!! (Ответ правильный!!!!)
Pat now has 1 Gold Coins. (Пэт зарабатывает 1 золотую монету)
Sue is the current player (Теперь на вопрос отвечает Сью)
They have rolled a 1 (Она выбросила 1)
Sue's new location is 1 (Ее текущее положение - 1)
The category is Science (Категория - Наука)
Science Question 0 (Вопрос категории Наука 0)
Answer was corrent!!!! (Ответ правильный!!!!)
Sue now has 1 Gold Coins. (Сью зарабатывает 1 золотую монету)
Chet is the current player (На вопрос отвечает Чет)
They have rolled a 4 (Он выбросил 4)

## Здесь мы удалили некоторые строки, ## чтобы не раздувать статью сверх всякой меры:

Answer was corrent!!!! (Ответ правильный!!!!)
Sue now has 5 Gold Coins. (Сью уже заработала 5 золотых монет)
Chet is the current player (Теперь на вопрос отвечает Чет)
They have rolled a 3 (Он выбросил 3)
Chet is getting out of the penalty box (Чет выходит из штрафной зоны)
Chet's new location is 11 (Его текущая позиция 11)
The category is Rock (Категория - Рок)
Rock Question 5 (Вопрос категории Рок 5)
Answer was correct!!!! (Ответ правильный!!!!)
Chet now has 5 Gold Coins. (Чет уже заработал 5 золотых монет)
Pat is the current player (Теперь на вопрос отвечает Пэт)
They have rolled a 1 (Он выбросил 1)
Pat's new location is 10 (Его текущая позиция 10)
The category is Sports (категория - Спорт)
Sports Question 1 (Вопрос категории Спорт 0)
Answer was corrent!!!! (Ответ правильный!!!!)
Pat now has 6 Gold Coins. (Пэт уже заработал 6 золотых монет)

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

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

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

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

Сканирование кода

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

Запуск викторины

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

будет представлено таким образом:

.. что уже немного лучше.

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

Глядя на файл GameRunner.php, мы можем легко определить некоторые ключевые аспекты, которые мы наблюдали при рассмотрении результата исполнения. Мы можем увидеть строки, которые добавляют пользователей (9-11), которые вызывают метод ходов roll() и определение победителя.

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

Файл викторины

Мы должны отформатировать аналогично и файл Game.php.

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

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

Золотой мастер

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

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

Так что же такое Золотой Мастер?

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

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

Рекомендуется запустить код, по крайней мере, 10 000 (десять тысяч) раз. Мы напишем тест, чтобы запустить код два раза и сохранить результат.

Написание генератора Золотого Мастера

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

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

Прилагаемый архив с кодами вы найдете в папке source, рядом с trivia нашей папки Test. В этой папке мы создаем файл: GoldenMasterTest.php:

class GoldenMasterTest extends PHPUnit_Framework_TestCase {

    function testGenerateOutput() {
        ob_start();
        require_once __DIR__ . '/../trivia/php/GameRunner.php';
        $output = ob_get_contents();
        ob_end_clean();

        var_dump($output);
    }

}

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

Код достаточно прост, он копирует в буфер выходные данные и помещает их в переменную $output. require_once() также будет исполнять весь код внутри включаемого файла. В нашей переменной для выгрузки данных мы увидим уже некоторые знакомые результаты:

Тем не менее, во время второго исполнения мы видим нечто странное:

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

Распределение генератора случайных чисел:

do {

    $aGame->roll(rand(0, 5) + 1);

    if (rand(0, 9) == 7) {
        $notAWinner = $aGame->wrongAnswer();
    } else {
        $notAWinner = $aGame->wasCorrectlyAnswered();
    }

} while ($notAWinner);

Анализируя исполнение функциональной части кода, мы видим, что в нем используется функция rand() для генерации случайных чисел. Нашим следующим шагом будет изучение официальной документации PHP, чтобы детальнее познакомиться с функцией rand().

"Генератор случайных чисел распределяет значения автоматически".

В документации говорится, что распределение значений происходит автоматически. Теперь у нас есть новая задача. Нам нужно найти способ контролировать распределение. Функция srand() может помочь нам в этом. Вот ее определение из документации.

"Устанавливает начальное число генератора случайных чисел в seed или случайное число, если seed не указан".

Это говорит нам, что если мы запустим эту функцию перед вызовом rand(), мы всегда в конечном итоге получим те же результаты:

function testGenerateOutput() {
    ob_start();
    srand(1);
    require_once __DIR__ . '/../trivia/php/GameRunner.php';
    $output = ob_get_contents();
    ob_end_clean();

    var_dump($output);
}

Мы вставляем srand(1) перед require_once(). Теперь результаты всегда будут одинаковыми:

Передаем выходные данные в файл:

class GoldenMasterTest extends PHPUnit_Framework_TestCase {

    function testGenerateOutput() {
        file_put_contents('/tmp/gm.txt', $this->generateOutput());
        $file_content = file_get_contents('/tmp/gm.txt');
        $this->assertEquals($file_content, $this->generateOutput());
    }

    private function generateOutput() {
        ob_start();
        srand(1);
        require_once __DIR__ . '/../trivia/php/GameRunner.php';
        $output = ob_get_contents();
        ob_end_clean();
        return $output;
    }

}

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

Причина заключается в том, что require_once() не будет обращаться к тому же файлу дважды. Второй вызов метода generateOutput() создаст пустую строку. И что же мы можем сделать? Что, если мы просто используем require()? Функция будет запускаться каждый раз:

Хорошо, но мы сталкиваемся с другой проблемой: "Cannot redeclare echoln()". Но откуда это берется? Из самого начала файла Game.php. Причина, по которой происходит эта ошибка, заключается в том, что в файле GameRunner.php содержится код include __DIR__ . '/Game.php';, который пытается включить файл викторины дважды, каждый раз, когда мы вызываем метод generateOutput():

include_once __DIR__ . '/Game.php';

Использование include_once в файле GameRunner.php решит эту проблему. Да, мы должны будем внести изменения в файл GameRunner.php еще до того, как создали тесты для него!

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

Запускаем тест несколько раз

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

function testGenerateOutput() {
    $this->generateMany(20, '/tmp/gm.txt');
    $this->generateMany(20, '/tmp/gm2.txt');
    $file_content_gm = file_get_contents('/tmp/gm.txt');
    $file_content_gm2 = file_get_contents('/tmp/gm2.txt');
    $this->assertEquals($file_content_gm, $file_content_gm2);
}

private function generateMany($times, $fileName) {
    $first = true;
    while ($times) {
        if ($first) {
            file_put_contents($fileName, $this->generateOutput());
            $first = false;
        } else {
            file_put_contents($fileName, $this->generateOutput(), FILE_APPEND);
        }
        $times--;
    }
}

Здесь мы извлекли другой метод: generateMany(). Он имеет два параметра. Один - количество раз, которое мы хотим запустить наш генератор, второй - файл, в который выгружаются результаты. Он указывает сгенерировать данные в файлы.

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

Но подождите. Один и тот же игрок выигрывает каждый раз? Разве это возможно?

cat /tmp/gm.txt | grep "has 6 Gold Coins."
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)

Да! Это возможно! Это более чем возможно. Так и должно быть. У нас один расклад для функции случайных чисел. Мы играем в ту же викторину снова и снова.

Запуск каждый раз по-разному

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

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

private function generateMany($times, $fileName) {
    $first = true;
    while ($times) {
        if ($first) {
            file_put_contents($fileName, $this->generateOutput($times));
            $first = false;
        } else {
            file_put_contents($fileName, $this->generateOutput($times), FILE_APPEND);
        }
        $times--;
    }
} 

private function generateOutput($seed) {
    ob_start();
    srand($seed);
    require __DIR__ . '/../trivia/php/GameRunner.php';
    $output = ob_get_contents();
    ob_end_clean();
    return $output;
}

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

cat /tmp/gm.txt | grep "has 6 Gold Coins."
Sue now has 6 Gold Coins. (Сью имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Pat now has 6 Gold Coins. (Пэт имеет 6 золотых монет.)
Pat now has 6 Gold Coins. (Пэт имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Sue now has 6 Gold Coins. (Сью имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Sue now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Sue now has 6 Gold Coins. (Сью имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Pat now has 6 Gold Coins. (Пэт имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)
Chet now has 6 Gold Coins. (Чет имеет 6 золотых монет.)

В произвольном порядке в викторине побеждают 3 различных участника. Это уже кое-что.

Доводим число запусков теста до 20000

Первое, что вы можете попробовать сделать, это запустить наш код для 20000 итераций викторины:

function testGenerateOutput() {
    $times = 20000;
    $this->generateMany($times, '/tmp/gm.txt');
    $this->generateMany($times, '/tmp/gm2.txt');
    $file_content_gm = file_get_contents('/tmp/gm.txt');
    $file_content_gm2 = file_get_contents('/tmp/gm2.txt');
    $this->assertEquals($file_content_gm, $file_content_gm2);
}

Это уже работает почти нормально. Будут сгенерированы два файла по 55MБайт:

ls -alh /tmp/gm*
-rw-r--r-- 1 csaba csaba 55M Mar 14 20:38 /tmp/gm2.txt
-rw-r--r-- 1 csaba csaba 55M Mar 14 20:38 /tmp/gm.txt

С другой стороны, тест проваливается из-за ошибки недостаточного объема памяти. И не имеет значения, сколько оперативной памяти в вашей системе - тест все равно не пройдет. У меня 8 ГБ плюс файл подкачки в 4ГБ, и ничего не получается. Две строки просто слишком велики, чтобы сравнить наши предположения:

Другими словами, мы создаем хорошие файлы, но PHPUnit не может их сравнить. Нам нужен обходной путь:

$this->assertFileEquals('/tmp/gm.txt', '/tmp/gm2.txt');

Это, кажется, может сработать, но тест по-прежнему не проходит. Какая досада. Мы должны придумать что-то еще:

$this->assertTrue($file_content_gm == $file_content_gm2);

Вот это, наконец, работает:

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

Таким образом, мы не сможем точно сказать, где проблема, если строки не совпадают. Мы лишь увидим надпись: "Failed asserting that false is true." Но с этим мы будем разбираться уже в следующей части серии.

Заключение

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

Да. Эти тесты очень медленные. На моей Core i7 машине генерация результатов дважды занимает 24 секунды. К счастью, в будущем мы не будем трогать файл gm.txt, а создадим еще один файл для одноразового запуска теста. Но 12 секунд по-прежнему очень много времени для такого небольшого кода.

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

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

09 июня 2014 в 19:07
Материалы по теме
{"url":"http://www.fastvps.ru/", "src":"/images/advbanners/fastvps.png", "alt":"Хостинг Fastvps.ru. Наш выбор!"}
Заработок