Pragmatic Perl

Выпуск 28. Июнь 2015

Рефакторинг Legacy

Рефакторинг устаревшего кода в примерах

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

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

Пусть у нас будет такой искуственный пример:

package Human;
use strict; use warnings;
sub do {
    my ($logger, $text, $key) = @_;
    if ($key eq 'rree') {
        # ... some code
        # normalize
        $text =~ s/[!\-\+]/ ! /g;
        $text =~ s/\(\(/ ( /g;
        $text =~ s/\)\)/ ); /g;
        if ($text =~ /superpower/) {
            $logger->log('warn', 'superpower used');
            $text =~ s/superpower/secret weapon/g;
        }
        # ... more normalize rules !!!
        # ... some code
        # prepare
        $text = sprintf '%s %s', $text, $key;
        unless ($text) {
            $logger->log('warning', 'no text');
        }
    }
    elsif ($key eq 'jjii') {
        # ...
    }
    return $text;
}
1;

Допустим, в блок кода, где мы нормализуем переменную $text, мне нужно добавить еще одну регулярку, например, такую:

$text =~ s/gg|rr|e|t|q//g;

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

Лучший друг рефакторинга это TDD — Test-driven development (разработка через тестирование), потому что мы хотим не просто сделать красиво, но чтобы оно еще и работало как раньше. Начнем с написания теста. Классически имя теста формируется по шаблону t-<MODULE>.t, поэтому наш тест назовем t-human.t.

И вот так он будет выглядеть:

  #!/usr/bin/perl
  use strict;
  use Test::More;
  use_ok('Human');
  done_testing();

Запустив в консоли:

$ prove t-human.t

Увидим такой результат:

t-human.t .. ok   
All tests successful.
Files=1, Tests=1,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.01 cusr  0.00 csys =  0.02 CPU)
Result: PASS

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

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

Теперь у нас такой код:

#... code
    if ($key ne 'rree') {
        # ... some code    
        # normalize
        my $text = _normalize($logger, $text);
        # ... some code
        # prepare
        $text = sprintf '%s %s', $text, $key;
        unless ($text) {
            $logger->log('warning', 'no text');
        }
    }
#... code
sub _normalize {
    my ($logger, $text) = @_;
    $text =~ s/[!\-\+]/ ! /g;
    $text =~ s/\(\(/ ( /g;
    $text =~ s/\)\)/ ); /g;
    if ($text =~ /superpower/) {
        $logger->log('warn', 'superpower used');
        $text =~ s/superpower/secret weapon/g;
    }
    # ... more normalize rules !!!
    return $text;
}

И проверяем:

$ prove t-human.t 

Тест до сих пор работает, показывая, что синтаксически все ОК.

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

my $tests = {
    '!abc+' => ' ! abc ! ',
    '((bbb))' => ' ( bbb ); ',
    'i want to use superpower' => 'i want to use secret weapon',
};

for my $t (keys %$tests) {
    is Human::_normalize($logger, $t), $test->{$t};
}

Но вот чертовщина! Функция имеет зависимость от неведомого $logger, у которого еще и методы вызывает. Просто передать undef не сработает. Здесь нужно себя перебороть и не удариться в невменяемый рефакторинг, пытаясь избавиться от этой зависимости. Пока будет достаточно просто вынести в функцию и на этом успокоиться. Но я не хочу писать сообщения в непонятный лог, я хочу их просто вывести в консоль и только в режиме -v.

"ref $logger" подсказывает, что это объект класса Logger. Ок, будем использовать его, но заменив метод log.

Делаем Mock с помощью Test::MockModule (сначала я использовала Test::Mock::Simple, но первый есть в libtest-mockmodule-perl, это определило выбор), и теперь наш фрагмент теста выглядит вот так:

# mock
my $module = Test::MockModule->new('Logger');
$module->mock('log', sub { shift; note explain ['IN MOCK', @_] }); 
my $logger = Logger->new();
for my $t (keys %$tests) {
    is Human::_normalize($logger, $t), $tests->{$t};
}

Здесь мы заменяем метод log объекта Logger простым выводом в консоль в режиме -v (метод note), а explain отдает человекопонятный дамп стека.

Запускаем тест:

$ prove t-human.t
t-human.t .. ok   
All tests successful.
Files=1, Tests=5,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.01 cusr  0.00 csys =  0.02 CPU)
Result: PASS

И вот так будет выглядеть вывод при использовании режима verbose:

$ prove t-human.t -v
t-human.t .. 
ok 1 - use Human;
ok 2 - use Logger;
# [
#   'IN MOCK',
#   'warn',
#   'superpower used'
# ]
ok 3
...

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

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

 my $tests = {
     '!abc+' => ' ! abc ! ',
     '((bbb))' => ' ( bbb ); ',
     'i want to use superpower' => 'i want to use secret weapon',
     'gg' => '',
     '!ggabcrr))' => ' ! abc ); ',
 };

Затем я всегда проверяю, что мой тест действительно перестал работать. Это паранойя? Но вдруг…

t-human.t .. 1/? 
#   Failed test at t-human.t line 23.
#          got: ' ! ggabcrr ); '
#     expected: ' ! abc ); '

#   Failed test at t-human.t line 23.
#          got: 'gg'
#     expected: ''
# Looks like you failed 2 tests of 7.

Дописываем логику в код:

sub _normalize {
    my ($logger, $text) = @_;
    $text =~ s/[!\-\+]/ ! /g;
    $text =~ s/\(\(/ ( /g;
    $text =~ s/\)\)/ ); /g;
    $text =~ s/gg|rr|e|t|q//g; # вот наше новое условие
    if ($text =~ /superpower/) {
        $logger->log('warn', 'superpower used');
        $text =~ s/superpower/secret weapon/g;
    }
    # ... more normalize rules
    return $text;
}

И наслаждаемся тем, что тесты проходят и все отлично! Ура.

$ prove t-human.t
t-human.t .. 1/? 
#   Failed test at t-human.t line 23.
#          got: 'i wan o us suprpowr'
#     expected: 'i want to use secret weapon'
# Looks like you failed 1 test of 7.

Ого, что-то пошло не так. Конечно! Наша новая логика вырезает слишком много всего. Немного подправим код:

if ($text =~ /superpower/) {
    $logger->log('warn', 'superpower used');
    $text =~ s/superpower/secret weapon/g;
}
else {
    $text =~ s/gg|rr|e|t|q//g; # перенесли сюда
}

$ prove t-human.t
t-human.t .. ok   
All tests successful.
Files=1, Tests=7,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.01 cusr  0.00 csys =  0.03 CPU)
Result: PASS

Теперь все ОК!

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

$ perl -MDevel::Cover t-human.t 
Devel::Cover: Writing coverage database to /home/wwax/Desktop/pragmatic/1/cover_db/runs/1431793338.4403.02108
----------------------------------- ------ ------ ------ ------ ------ ------
File                                  stmt   bran   cond    sub   time  total
----------------------------------- ------ ------ ------ ------ ------ ------
Human.pm                              62.5   25.0    n/a   75.0    1.3   55.5
Logger.pm                            100.0    n/a    n/a  100.0    0.0  100.0
t-human.t                            100.0    n/a    n/a  100.0   98.6  100.0
Total                                 81.2   25.0    n/a   90.0  100.0   75.7
----------------------------------- ------ ------ ------ ------ ------ ------

Нам важна только первая строка. Неплохие результаты, учитывая, что до этого никакого тестирования там вообще не было. Если в модуле Human.pm оставить только единственную функцию _normalize, ради которой это все затевалось, то cover показывает 100% =). Это значит, что все функции, операторы и ветвления были пройдены в результате нашего теста.

----------------------------------- ------ ------ ------ ------ ------ ------
File                                  stmt   bran   cond    sub   time  total
----------------------------------- ------ ------ ------ ------ ------ ------
Human.pm                             100.0  100.0    n/a  100.0    1.2  100.0

Успешного рефакторинга! И я надеюсь, будет продолжение =)

■ Наталья Савенкова