Статьи

Рефакторинг унаследованного кода. Часть 10. Разбор длинных методов с извлечением

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

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


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

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

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

Давайте посмотрим на пример случайного кода из Интернета. Stackoverflow — хорошее место для поиска небольших кусочков кода. Вот тот, который определяет , является ли число простым :

1
//Check if a number is prime function isPrime($num, $pf = null) { if(!is_array($pf)) { for($i=2;$i<intval(sqrt($num));$i++) { if($num % $i==0) { return false;

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

Согласно Википедии:

Простое число (или простое число) — это натуральное число, большее 1, которое не имеет положительных делителей, кроме 1 и самого себя.

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

01
02
03
04
05
06
07
08
09
10
11
12
13
class IsPrimeTest extends PHPUnit_Framework_TestCase {
 
    function testItCanRecognizePrimeNumbers() {
        $this->assertTrue(isPrime(1));
    }
 
}
 
// Check if a number is prime
function isPrime($num, $pf = null)
{
    // … the content of the method as seen above
}

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

Этот тест проходит. Мой следующий инстинкт — добавить еще несколько простых чисел, а затем написать еще один тест с не простыми числами.

1
2
3
4
5
6
7
8
function testItCanRecognizePrimeNumbers() {
    $this->assertTrue(isPrime(1));
    $this->assertTrue(isPrime(2));
    $this->assertTrue(isPrime(3));
    $this->assertTrue(isPrime(5));
    $this->assertTrue(isPrime(7));
    $this->assertTrue(isPrime(11));
}

Это проходит. Но как насчет этого?

1
2
3
function testItCanRecognizeNonPrimes() {
    $this->assertFalse(isPrime(6));
}

Это неожиданно не получается: 6 не простое число. Я ожидал, что метод вернет false . Я не знаю, как работает метод или назначение параметра $pf — я просто ожидал, что он вернет false основываясь на его имени и описании. Я понятия не имею, почему это не работает, и как это исправить.

Это довольно запутанная дилемма. Что нам делать? Лучший ответ — написать тесты, которые пройдут приличный объем цифр. Возможно, нам нужно попытаться угадать, но, по крайней мере, у нас будет представление о том, что делает метод. Тогда мы можем начать рефакторинг.

1
2
3
4
5
function testFirst20NaturalNumbers() {
    for ($i=1;$i<20;$i++) {
        echo $i .
    }
}

Это выводит что-то интересное:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
1 — true
2 — true
3 — true
4 — true
5 — true
6 — true
7 — true
8 — true
9 — true
10 — false
11 — true
12 — false
13 — true
14 — false
15 — true
16 — false
17 — true
18 — false
19 — true

Шаблон начинает появляться здесь. Все верно до 9, затем чередуется до 19. Но повторяется ли этот паттерн? Попробуйте запустить его на 100 номеров, и вы сразу увидите, что это не так. На самом деле кажется, что он работает для чисел от 40 до 99. Он пропускает один раз между 30-39, назначая 35 как простое число. То же самое верно в диапазоне 20-29. 25 считается простым.

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

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

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

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

1
2
3
4
5
function testGenerateGoldenMaster() {
    for ($i=1;$i<10000;$i++) {
        file_put_contents(__DIR__ . ‘/IsPrimeGoldenMaster.txt’, $i . ‘ — ‘ . (isPrime($i) ? ‘true’ : ‘false’) . «\n», FILE_APPEND);
    }
}

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

1
2
3
4
5
6
7
function testMatchesGoldenMaster() {
    $goldenMaster = file(__DIR__ . ‘/IsPrimeGoldenMaster.txt’);
    for ($i=1;$i<10000;$i++) {
        $actualResult = $i .
        $this->assertTrue(in_array($actualResult, $goldenMaster), ‘The value ‘ . $actualResult . ‘ is not in the golden master.’);
    }
}

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

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
class IsPrimeTest extends PHPUnit_Framework_TestCase {
 
    function testGenerateGoldenMaster() {
        $this->markTestSkipped();
        for ($i=1;$i<10000;$i++) {
            file_put_contents(__DIR__ . ‘/IsPrimeGoldenMaster.txt’, $this->getPrimeResultAsString($i), FILE_APPEND);
        }
    }
 
    function testMatchesGoldenMaster() {
        $goldenMaster = file(__DIR__ . ‘/IsPrimeGoldenMaster.txt’);
        for ($i=1;$i<10000;$i++) {
            $actualResult = $this->getPrimeResultAsString($i);
            $this->assertTrue(in_array($actualResult, $goldenMaster), ‘The value ‘ . $actualResult . ‘ is not in the golden master.’);
        }
    }
 
    private function getPrimeResultAsString($i) {
        return $i .
    }
}

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

Сначала мы можем извлечь метод isDivisible() в первой части кода.

1
2
3
4
5
6
7
8
9
if(!is_array($pf))
{
    for($i=2;$i<intval(sqrt($num));$i++) {
        if(isDivisible($num, $i)) {
            return false;
        }
    }
    return true;
}

Это позволит нам повторно использовать код во второй части следующим образом:

1
2
3
4
5
6
7
8
9
} else {
    $pfCount = count($pf);
    for($i=0;$i<$pfCount;$i++) {
        if(isDivisible($num, $pf[$i])) {
            return false;
        }
    }
    return true;
}

И как только мы начали работать с этим кодом, мы заметили, что он небрежно выровнен. Брекеты иногда в начале строки, а другие в конце.

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

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
//Check if a number is prime
function isPrime($num, $pf = null) {
    if (!is_array($pf)) {
        for ($i = 2; $i < intval(sqrt($num)); $i++) {
            if (isDivisible($num, $i)) {
                return false;
            }
        }
        return true;
    } else {
        $pfCount = count($pf);
        for ($i = 0; $i < $pfCount; $i++) {
            if (isDivisible($num, $pf[$i])) {
                return false;
            }
        }
        return true;
    }
}

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

Если извлеченный метод вернет логическое значение, и мы сравним его, чтобы решить, следует ли нам возвращаться из isPrime() , это не поможет вообще. Может быть способ извлечь его, используя некоторые концепции функционального программирования в PHP , но, возможно, позже. Сначала мы можем сделать что-то попроще.

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
function isPrime($num, $pf = null) {
    if (!is_array($pf)) {
        return checkDivisorsBetween(2, intval(sqrt($num)), $num);
    } else {
        $pfCount = count($pf);
        for ($i = 0; $i < $pfCount; $i++) {
            if (isDivisible($num, $pf[$i])) {
                return false;
            }
        }
        return true;
    }
}
 
function checkDivisorsBetween($start, $end, $num) {
    for ($i = $start; $i < $end; $i++) {
        if (isDivisible($num, $i)) {
            return false;
        }
    }
    return true;
}

Извлечь цикл for в целом немного проще, но когда мы попытаемся повторно использовать наш извлеченный метод во второй части if мы увидим, что он не будет работать. Есть таинственная переменная $pf о которой мы почти ничего не знаем.

Похоже, что он проверяет, делится ли число на набор конкретных делителей, вместо того, чтобы брать все числа до другого магического значения, определенного intval(sqrt($num)) . Может быть, мы могли бы переименовать $pf в $divisors .

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
function isPrime($num, $divisors = null) {
    if (!is_array($divisors)) {
        return checkDivisorsBetween(2, intval(sqrt($num)), $num);
    } else {
        return checkDivisorsBetween(0, count($divisors), $num, $divisors);
    }
}
 
function checkDivisorsBetween($start, $end, $num, $divisors = null) {
    for ($i = $start; $i < $end; $i++) {
        if (isDivisible($num, $divisors ? $divisors[$i] : $i)) {
            return false;
        }
    }
    return true;
}

Это один из способов сделать это. Мы добавили четвертый необязательный параметр в наш метод проверки. Если оно имеет значение, мы используем его, в противном случае мы используем $i .

Можем ли мы извлечь что-нибудь еще? Как насчет этого фрагмента кода: intval(sqrt($num)) ?

01
02
03
04
05
06
07
08
09
10
11
function isPrime($num, $divisors = null) {
    if (!is_array($divisors)) {
        return checkDivisorsBetween(2, integerRootOf($num), $num);
    } else {
        return checkDivisorsBetween(0, count($divisors), $num, $divisors);
    }
}
 
function integerRootOf($num) {
    return intval(sqrt($num));
}

Разве это не лучше? В некотором роде. Лучше, если человек, идущий за нами, не знает, что intval() и sqrt() , но это не помогает облегчить понимание логики. Почему мы заканчиваем цикл for с этим конкретным номером? Может быть, это вопрос, на который должно ответить наше имя функции.

01
02
03
04
05
06
07
08
09
10
11
12
[PHP]//Check if a number is prime
function isPrime($num, $divisors = null) {
    if (!is_array($divisors)) {
        return checkDivisorsBetween(2, highestPossibleFactor($num), $num);
    } else {
        return checkDivisorsBetween(0, count($divisors), $num, $divisors);
    }
}
 
function highestPossibleFactor($num) {
    return intval(sqrt($num));
}[PHP]

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

Вопрос в том, можем ли мы извлечь что-то еще? Ну, мы должны попытаться, пока мы не упадем. Я упомянул сторону функционального программирования PHP несколькими параграфами выше. Есть две основные характеристики функционального программирования, которые мы можем легко применить в PHP: функции первого класса и рекурсия. Всякий раз, когда я вижу оператор if с return внутри цикла for , как в нашем checkDivisorsBetween() , я думаю о применении одного или обоих методов.

1
2
3
4
5
6
7
8
function checkDivisorsBetween($start, $end, $num, $divisors = null) {
    for ($i = $start; $i < $end; $i++) {
        if (isDivisible($num, $divisors ? $divisors[$i] : $i)) {
            return false;
        }
    }
    return true;
}

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

01
02
03
04
05
06
07
08
09
10
11
function checkDivisorsBetween($start, $end, $num, $divisors = null) {
    $numberIsNotPrime = function ($num, $divisor) {
        if (isDivisible($num, $divisor)) {
            return false;
        }
    };
    for ($i = $start; $i < $end; $i++) {
        $numberIsNotPrime($num, $divisors ? $divisors[$i] : $i);
    }
    return true;
}

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

1
2
3
4
5
function checkRecursiveDivisibility($current, $end, $num, $divisor) {
    if($current == $end) {
        return true;
    }
}

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

1
2
3
4
5
6
7
8
9
function checkRecursiveDivisibility($current, $end, $num, $divisor) {
    if($current == $end) {
        return true;
    }
 
    if (isDivisible($num, $divisor)) {
        return false;
    }
}

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

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
ini_set(‘xdebug.max_nesting_level’, 10000);
function checkDivisorsBetween($start, $end, $num, $divisors = null) {
    return checkRecursiveDivisibility($start, $end, $num, $divisors);
}
 
function checkRecursiveDivisibility($current, $end, $num, $divisors) {
    if($current == $end) {
        return true;
    }
 
    if (isDivisible($num, $divisors ? $divisors[$current] : $current)) {
        return false;
    }
 
    checkRecursiveDivisibility($current++, $end, $num, $divisors);
}

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


Когда я писал «Золотой мастер», я специально что-то упустил. Давайте просто скажем, что тесты не покрывают столько кода, сколько они должны. Можете ли вы определить проблему? Если да, как бы вы подошли к этому?


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

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