Статьи

Рефакторинг устаревшего кода — часть 11: конец?

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

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


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

Как обычно, у вас есть код, который был, когда я запускал этот урок в папке php_start , а конечный результат — в папке php .

Этот метод wasCorrectlyAnswered() является нашей первой жертвой.


Как мы узнали из предыдущих уроков, первый шаг при изменении устаревшего кода — это его тестирование. Это может быть сложным процессом. К счастью для нас, метод wasCorrectlyAnswered() довольно прост. Он состоит из нескольких операторов if-else . Каждая ветвь кода возвращает значение. Когда у нас есть возвращаемое значение, мы всегда можем предположить, что тестирование выполнимо. Не обязательно просто, но по крайней мере возможно.

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

Это легче читать и значительно более наглядно. Вы можете найти извлеченные методы в прикрепленном коде.

Мы ожидали, что это пройдет, но это не удалось. Причины не ясны вообще. Более внимательный взгляд на didPlayerNotWin() может быть полезным.

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

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

Это работает При анализе didPlayerNotWin() мы также заметили, что он использует равенство для определения победителя. Мы должны установить наше значение на единицу меньше, потому что значение увеличивается в производственном коде, который мы тестируем.

Оставшиеся три теста просто написать. Это всего лишь вариации первых двух. Вы можете найти их в прикрепленном коде.


Наиболее запутанная проблема — вводящее в заблуждение имя переменной $winner . Это должно быть $notAWinner .

Мы можем заметить, что переменная $notAWinner используется только для возврата значения. Можем ли мы вызвать метод didPlayerNotWin() непосредственно в операторе возврата?

Это все еще проходит наш модульный тест, но если мы запустим наши тесты Golden Master, они потерпят неудачу с ошибкой «недостаточно памяти». Фактически, изменение делает игру никогда не законченной.

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

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

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

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

Как правило, старайтесь иметь одну строку кода на каждом пути логики принятия решения.

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

И если мы сможем избавиться от лишнего кода, мы должны. Удалив else и сохранив логику, мы немного сэкономили. Мне больше нравится это решение, потому что оно подчеркивает поведение функции по умолчанию. Код, который находится непосредственно внутри функции (последняя строка кода здесь). Оператор if является исключительной функциональностью, добавленной к стандартному.

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

Мы можем продолжать извлекать внутри наших недавно созданных частных методов. Применение того же принципа к нашему следующему условному утверждению приводит к приведенному выше коду.

Глядя на наши закрытые методы getCorrectlyAnsweredForPlayersNotInPenaltyBox() и getCorrectlyAnsweredForPlayerGettingOutOfPenaltyBox() мы сразу можем заметить, что простое назначение дублируется. Это назначение может быть очевидным для кого-то вроде нас, который уже знает, что с кошельками и монетами, но не для новичка. Извлечение этой единственной строки в метод giveCurrentUserACoin() является хорошей идеей.

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

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

На этом работа над wasCorrectlyAnswered() .


На 11 строчках этот метод не огромный, но, безусловно, большой. Вы помните магическое число семь плюс минус два исследования? В нем говорится, что наш мозг может одновременно думать о 7 + -2 вещах. То есть у нас ограниченные возможности. Поэтому, чтобы легко и полностью понять метод, мы хотим, чтобы логика внутри него вписывалась в этот диапазон. В общей сложности 11 строк и 9 строк, этот метод является своего рода пределом. Вы можете утверждать, что на самом деле есть пустая строка, а другая — только с фигурной скобкой. Это сделало бы только 7 строк логики.

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

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

У нас уже есть метод для этого куска кода, поэтому мы должны его использовать.

Лучше, но мы должны отказаться или продолжить?

Мы могли бы вставить переменную из этих двух строк. $this->currentPlayer , очевидно, возвращает текущего игрока, поэтому не нужно повторять логику. Мы не узнаем ничего нового или ничего абстрактного с помощью локальной переменной.

Мы до 5 строк. Что-нибудь еще там?

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

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


Давайте сделаем немного статистики, проверив этот замечательный инструмент от автора PHPUnit https://github.com/sebastianbergmann/phploc.git

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
./phploc ../Refactoring\ Legacy\ Code\ -\ Part\ 1\:\ The\ Golden\ Master/Source/trivia/php/
phploc 2.1-gca70e70 by Sebastian Bergmann.
 
Size
  Lines of Code (LOC) 232
  Comment Lines of Code (CLOC) 0 (0.00%)
  Non-Comment Lines of Code (NCLOC) 232 (100.00%)
  Logical Lines of Code (LLOC) 99 (42.67%)
    Classes 88 (88.89%)
      Average Class Length 88
        Minimum Class Length 88
        Maximum Class Length 88
      Average Method Length 7
        Minimum Method Length 1
        Maximum Method Length 17
    Functions 1 (1.01%)
      Average Function Length 1
    Not in classes or functions 10 (10.10%)
 
Cyclomatic Complexity
  Average Complexity per LLOC 0.26
  Average Complexity per Class 25.00
    Minimum Class Complexity 25.00
    Maximum Class Complexity 25.00
  Average Complexity per Method 3.18
    Minimum Method Complexity 1.00
    Maximum Method Complexity 10.00
 
Dependencies
  Global Accesses 0
    Global Constants 0 (0.00%)
    Global Variables 0 (0.00%)
    Super-Global Variables 0 (0.00%)
  Attribute Accesses 115
    Non-Static 115 (100.00%)
    Static 0 (0.00%)
  Method Calls 21
    Non-Static 21 (100.00%)
    Static 0 (0.00%)
 
Structure
  Namespaces 0
  Interfaces 0
  Traits 0
  Classes 1
    Abstract Classes 0 (0.00%)
    Concrete Classes 1 (100.00%)
  Methods 11
    Scope
      Non-Static Methods 11 (100.00%)
      Static Methods 0 (0.00%)
    Visibility
      Public Methods 11 (100.00%)
      Non-Public Methods 0 (0.00%)
  Functions 1
    Named Functions 1 (100.00%)
    Anonymous Functions 0 (0.00%)
  Constants 0
    Global Constants 0 (0.00%)
    Class Constants 0 (0.00%)

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

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

Как только мы посмотрим на среднюю длину класса, мы увидим резкое снижение количества строк кода с 88 до 36.

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

Хотя количество строк является хорошим индикатором объема кода на единицу измерения, реальный выигрыш заключается в анализе цикломатической сложности. Каждый раз, когда мы принимаем решение в нашем коде, мы увеличиваем цикломатическую сложность. Когда мы соединяем операторы if внутри другого, цикломатическая сложность этого метода возрастает экспоненциально. Наши постоянные извлечения привели к методам, в которых принималось только одно решение, что снизило их среднюю сложность для метода с 3,18 до 1,00. Вы можете прочитать это как «наши рефакторинговые методы в 3,18 раза проще, чем исходный код». На уровне класса падение сложности еще более удивительно. Он снизился с 25.00 до 6.50.

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

Спасибо за ваше пристальное внимание.