Govnokodu.Net

Тотальный рефакторинг короткого кода из трёх строк

Исходный код

for($i=0;$i<10;$i++){
    $tf2[rand(0,count($tf2)-1)][2]-=$tf1[rand(0,count($tf1)-1)][1];
    $tf1[rand(0,count($tf1)-1)][2]-=$tf2[rand(0,count($tf2)-1)][1];
}

Что не так в исходном коде

Вопросы к коду

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

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

Что такое $tf1 и $tf2 — что такое каждый элемент в этих массивах? И что такое эти элементы в каждом контексте? Что такое элементы 2 и 1 внутри тех элементов? Что происходит в каждой итерации? Почему итераций именно 10?

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

Идентификаторы

  1. Литерал 10 можно заменить на константу, и в её названии прописать смысл действий, происходящих на каждой итерации.
  2. В названиях массивов $tf1 и $tf2 стоит разъяснить, чем является совокупность объектов, заключенных в этих массивах (вся совокупность в целом, а не каждый отдельный элемент).
  3. Под элемент массива стоит создать отдельный класс, а в его названии прописать, чем этот класс является.
  4. Операцию -= следует спрятать в метод, а название методу дать в соответствии со смыслом этого вычитания. А в названии параметра — роль объекта в контексте операции.

Смыслы объектов и операций не придумываются с потолка, а возникают из предметной области, которую моделирует соответствующий код.

Проклятие стандартных типов данных

Нумерованный массив вместо объекта

Пренебрежение идентификаторами отчасти вызвано самими возможностями языка: не проблема засунуть в нумерованный массив элементы разных типов. Жить потом с этим — та ещё проблема.

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

В исходном коде элемент массива $tf2[rand(0,count($tf2)-1)], который сам является нумерованным массивом, по сути должен реализовываться как структура с именованными полями. Только название поля может отразить его смысл, никак не номер.

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

Анемичные коллекции

Другая проблема со стандартными типами данных вызвана тем обстоятельством, что на практичке почти всегда коллекция объектов не является тупым объединением объектов в набор — у этого набора есть своё специфическое поведение.

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

В контексте исходного кода такой операцией является «дай мне случайный объект».

Его код выглядит как $tf2[rand(0,count($tf2)-1)], и этот код вполне достоин извлечения в отдельный метод (с одновременным созданием нового класса).

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

Смешивание знаний

Конечно же, нельзя не упомянуть о смешивании нескольких независимых знаний:

  • как выбирать элементы из массивов
  • какими элементами элементов выполнять операцию и какую
  • что происходит на каждой итерации цикла

Смешивание уровней

В исходном коде не просто смешиваются различные знания — они смешиваются на разных уровнях. Например, операция [rand(0,count($tf2)-1)] относится к коллекции, [1] — к элементу коллекции, а -= — к полю элемента коллекции. Кроме того, цикл относится к чему-то, что «выше» коллекции. Фактически, в каких-то трёх коротких строках кода смешаны операции четырёх уровней.

Пример рефакторинга исходного кода

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

class Fighter extends AbstractFighter {
    private $strength;
    private $power;

    public function hitBy(Fighter $attacker) {
        $this->strength -= $attacker->getPower();
    }
    public function getPower() {
        return $power;
    }
}
class Army {
    private $fighters;

    static public function fight($army1, $army2, $rounds) {
        for ($i = 1; $i <= $rounds; $i++) {
            $army1->hitBy($army2);
            $army2->hitBy($army1);
        }
    }
    public function hitBy(Army $attacking_army) {
        $this->getDefender()->hitBy($attacking_army->getAttacker());
    }
    private function getAttacker() {
        return $this->randomFighter();
    }
    private function getDefender() {
        return $this->randomFighter();
    }
    private function randomFighter() {
        return $this->fighters[random(0, count($this->fighters) - 1)];
    }
}
const FIGHTING_ROUND_COUNT = 10;\
Army::fight($army1, $army2, FIGHTING_ROUND_COUNT);

Теория

  • Название переменной
  • Проклятие стандартных типов данных
  • Анемичные коллекции
  • Принцип единственного знания
  • Принцип единственного уровня
  • Рефакторинг «Замена значения данных объектом»
  • Рефакторинг «Замена массива объектом»
  • Рефакторинг «Инкапсуляция коллекции»
  • Рефакторинг «Извлечение класса»
  • Рефакторинг «Извлечение метода»
  • Рефакторинг «Замена магического числа символической константой»
  • Рефакторинг «Замена магического числа вызовом метода»

Сергей Галанин