Запахи кода и каталог рефакторингов

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

1. Как заметить, что код уже не такой, каким должен быть, — до того как это заметит инцидент в проде.
2. Как его починить так, чтобы поведение не изменилось, а работа не встала на две недели.

Первое — это запахи кода. Второе — рефакторинг. Вместе они образуют единственную практику из всего трека, которую можно выполнять механически и почти без творчества: увидел паттерн проблемы → применил известную последовательность шагов → проверил тестами → закоммитил. Именно поэтому рефакторинг так хорошо ложится на автоматизацию в IDE и так плохо — на «мы выделим спринт и всё перепишем».

1. Рефакторинг: строгое определение и почему оно важно

Слово «рефакторинг» в индустрии затёрли до состояния «любые изменения в коде, за которые не платит заказчик». Мартин Фаулер в книге Refactoring: Improving the Design of Existing Code даёт узкое определение, и от узости здесь вся польза:

Рефакторинг (сущ.) — изменение внутренней структуры программного обеспечения, облегчающее его понимание и удешевляющее модификацию, без изменения наблюдаемого поведения.

Рефакторить (гл.) — изменять структуру ПО, применяя последовательность рефакторингов, без изменения наблюдаемого поведения.

Ключевые слова — «наблюдаемое поведение» и «последовательность». Разберём оба.

Наблюдаемое поведение не меняется. Это не значит «программа делает то же самое побайтово»: производительность может измениться, порядок логов может измениться, внутренние приватные структуры точно изменятся. Не должно измениться то, что видит клиент кода — контракт: результаты для валидных входов, ошибки для невалидных, наблюдаемые побочные эффекты. Отсюда следует главный практический вывод: рефакторинг и изменение поведения нельзя смешивать в одном коммите. Это две разные «шляпы» (Фаулер называет это two hats): в шляпе рефакторинга вы не добавляете функциональность и не пишете новых тестов; в шляпе фичи — не двигаете код.

Последовательность маленьких шагов. Рефакторинг — это не «переписать модуль на выходных». Каждый отдельный рефакторинг — это преобразование на 30 секунд–5 минут, после которого код снова компилируется и тесты снова зелёные. Большое изменение архитектуры получается как композиция из сотни таких шагов, и в любой момент этой сотни вы можете остановиться и уйти домой с рабочей системой.

Что рефакторингом не является:

Последняя строка — самая частая ошибка в командах. Диф на 800 строк, где вперемешку переименования, переносы файлов и новая бизнес-логика, физически невозможно отревьюить: ревьюер сдаётся и пишет LGTM. Про то, как это ловится на процессе, — в статье Код-ревью, командные стандарты и Definition of Done.

Экономика: технический долг

Уорд Каннингем в 1992 году предложил метафору долга: выпустить недоработанный код — это как взять кредит. Иногда это разумно (быстрее выйти на рынок, быстрее получить обратную связь), но по кредиту капают проценты — в виде замедления каждой следующей фичи. Рефакторинг — это платёж по телу долга.

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

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

2. Запах кода: что это и что это не

Термин code smell придумал Кент Бек, а в оборот ввела глава 3 книги Фаулера. Определение намеренно нестрогое:

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

Три слова здесь несут всю нагрузку:

• Поверхностный — запах виден локально, без понимания всей системы. Функция на 300 строк видна сразу; «нарушение SRP» надо доказывать.
• Часто, а не всегда — запах это не диагноз, а повод присмотреться. Функция на 300 строк, состоящая из плоской таблицы констант, — прекрасный код.
• Более глубокая проблема — сам по себе запах не вреден, вреден дизайн, на который он намекает.

Ценность запахов ровно в том, что они переводят разговор о качестве кода из «мне не нравится» в «здесь Feature Envy: метод Order.print() шесть раз обращается к полям Customer». Второе обсуждаемо, первое — вкусовщина. Это делает запахи главным языком код-ревью.

Хороший бесплатный каталог с примерами на нескольких языках — refactoring.guru/refactoring/smells; канонический источник — глава 3 у Фаулера (во втором издании 24 запаха).

Таксономия

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

Дальше разберём семейства по очереди — с кодом и с конкретным рецептом лечения.

3. Каталог запахов с примерами

3.1. Раздувальщики (bloaters)

Код, который рос постепенно и никто не остановился.

Длинная функция (Long Method). Самый частый и самый вредный запах. Порог «сколько строк много» бессмысленно задавать числом; рабочий критерий Фаулера: если внутри функции вам захотелось написать комментарий, объясняющий, что делает следующий блок, — этот блок должен стать функцией с таким именем. Второй критерий, из Clean Code Роберта Мартина: функция должна работать на одном уровне абстракции. Смесь if order.total > limit и cursor.execute("INSERT ...") в одном теле — верный признак.

Лечение: Extract Function, и почти всегда следом — разделение фаз (Split Phase): вычисления отдельно, ввод-вывод отдельно.

Длинный список параметров (Long Parameter List).

# ЗАПАХ: восемь позиционных параметров, три из которых булевы флаги
def create_report(user_id, date_from, date_to, currency,
                  include_taxes, include_refunds, group_by_day, fmt):
    ...

create_report(42, d1, d2, "EUR", True, False, True, "pdf")  # что означает True?

Булевы флаги в сигнатуре — отдельный подзапах (flag argument): вызов нечитаем, а тело функции почти наверняка содержит if include_taxes: — то есть функция делает два разных дела. Лечение — Introduce Parameter Object и/или Remove Flag Argument (расщепить на две функции с говорящими именами):

from dataclasses import dataclass
from datetime import date

@dataclass(frozen=True)
class ReportSpec:
    """Параметры отчёта как единое понятие домена, а не россыпь аргументов."""
    period: "DateRange"
    currency: str
    include_taxes: bool = True
    include_refunds: bool = False
    group_by_day: bool = False

def render_pdf_report(spec: ReportSpec) -> bytes: ...
def render_csv_report(spec: ReportSpec) -> bytes: ...

Выигрыш не только в читаемости: у ReportSpec появилось место, куда положить валидацию (period.start <= period.end) — а раньше её приходилось дублировать в каждом вызывающем.

Группы данных (Data Clumps). Одни и те же 3–4 значения кочуют вместе по сигнатурам: (amount, currency), (lat, lon), (start, end). Тест: мысленно удалите одно поле — остальные теряют смысл? Значит, это одно понятие, у которого нет типа.

Одержимость примитивами (Primitive Obsession). Деньги — float, email — str, идентификатор пользователя — int. Пока всё в примитивах, компилятор не помогает: transfer(user_id, order_id) перепутать местами ничто не мешает.

from decimal import Decimal
from typing import NewType

UserId = NewType("UserId", int)   # дёшево: ноль рантайм-стоимости, но mypy ловит путаницу
OrderId = NewType("OrderId", int)

@dataclass(frozen=True)
class Money:
    """Деньги никогда не float: 0.1 + 0.2 != 0.3, а на биллинге это инцидент."""
    amount: Decimal
    currency: str

    def __add__(self, other: "Money") -> "Money":
        if self.currency != other.currency:
            raise ValueError(f"нельзя складывать {self.currency} и {other.currency}")
        return Money(self.amount + other.amount, self.currency)

Money заодно убивает целый класс багов «сложили рубли с евро» — раньше это была тихая арифметика, теперь исключение. Это общий эффект лечения Primitive Obsession: вы переносите ошибки из рантайма прода в момент компиляции или в момент первого теста.

3.2. Препятствия изменению (change preventers)

Самое дорогое семейство: оно прямо повышает C_find и C_verify из формулы стоимости изменения, разобранной в обзоре трека.

Расходящиеся модификации (Divergent Change). Один модуль меняется по многим не связанным между собой причинам. «Файл order_service.py трогают и когда меняется НДС, и когда меняется шаблон письма, и когда добавляют платёжного провайдера» — это нарушение SRP в его исходной формулировке через «причины для изменения» (см. SOLID).

Стрельба дробью (Shotgun Surgery). Зеркальный запах: одно изменение требует правок в дюжине файлов. Добавили валюту — правьте enum, маппинг в БД, три форматтера, конфиг и два фронтенда. Причина — знание размазано, а должно быть в одном месте.

Отличать их важно, потому что лечение противоположно:

Ветка G здесь принципиальна. Если после двух-трёх попыток собрать логику в одном месте она всё равно расползается, значит, границы модулей не совпадают с границами понятий в домене. Никакой Move Function этого не починит — нужен разговор с продуктом и пересмотр модели.

3.3. Нарушители ООП (OO abusers)

Switch по типу. Классика:

# ЗАПАХ: switch по «типу» объекта, продублированный в нескольких местах системы
def pay(order):
    if order.method == "card":
        return charge_card(order)
    elif order.method == "sbp":
        return charge_sbp(order)
    elif order.method == "invoice":
        return issue_invoice(order)
    raise ValueError(order.method)

def receipt_line(order):     # тот же switch, второй экземпляр
    if order.method == "card":
        return "Оплата картой"
    elif order.method == "sbp":
        return "Оплата по СБП"
    ...

Один switch — не запах. Запахом он становится, когда тот же самый набор ветвлений повторяется в разных местах: добавление способа оплаты превращается в Shotgun Surgery, а забытая ветка — в рантайм-ошибку. Лечение — Replace Conditional with Polymorphism:

Теперь добавление способа оплаты — один новый файл и одна строка в фабрике; компилятор (или ABC в Python) заставит реализовать все методы.

Осторожно, trade-off. Полиморфизм не бесплатен: логика одного способа оплаты собирается вместе, но логика одной операции («как выглядит строка чека для всех способов») размазывается по классам. Это expression problem в чистом виде:

• часто добавляются новые типы при стабильном наборе операций → полиморфизм/классы выигрывают;
• часто добавляются новые операции при стабильном наборе типов → выигрывает switch/pattern matching (и в Python 3.10+ match с исчерпывающей проверкой у mypy, и в Rust/Scala/F#).

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

Отказ от наследства (Refused Bequest). Подкласс наследует метод и реализует его как raise NotImplementedError. Это прямое нарушение LSP; лечение — заменить наследование делегацией (Replace Superclass with Delegate).

Временное поле (Temporary Field). Поле объекта, заполненное только во время выполнения одного длинного метода, — верный признак того, что этот метод хочет быть отдельным классом (Extract Class, часто в форме Method Object).

3.4. Мусор (dispensables)

Дублирование кода. Самый известный запах и самый неверно применяемый: подробный разбор, когда дублирование лечить, а когда терпеть, — в статье DRY, KISS, YAGNI. Коротко: дублирование знания лечим, дублирование текста при разных причинах изменения — оставляем.

Спекулятивная общность (Speculative Generality). Абстракция, добавленная «на будущее»: интерфейс с единственной реализацией, параметр, который везде передают одинаковым, хук, который никто не переопределяет. Лечение — Inline Class, Collapse Hierarchy, Remove Parameter. Практический критерий: если у абстракции меньше двух реальных потребителей, она не абстракция, а лишний прыжок при чтении.

Комментарий как дезодорант. Комментарий сам по себе не запах, но комментарий, объясняющий что делает нечитаемый блок, — почти всегда несделанный Extract Function. Разбор того, какие комментарии полезны (а полезные точно есть: «почему», инварианты, ссылки на тикеты и стандарты), — в статье Чистый код.

Мёртвый код. Удаляйте, не комментируйте. У вас есть git; закомментированный блок — это код, который не компилируется, не тестируется, но продолжает участвовать в чтении и в поиске по репозиторию.

3.5. Сцепщики (couplers)

Это семейство целиком про то, чему посвящена статья Связанность, связность и закон Деметры.

Завистливые функции (Feature Envy). Метод интересуется данными чужого класса больше, чем своими:

# ЗАПАХ: Invoice ничего не знает о себе, зато знает всё о Customer
class Invoice:
    def discount(self, customer):
        if customer.tier == "gold" and customer.years >= 3 and not customer.has_debt:
            return Decimal("0.15")
        if customer.tier == "silver" and customer.years >= 1:
            return Decimal("0.07")
        return Decimal("0")

Лечение — Move Function: правило скидки принадлежит Customer, потому что оно целиком выражено через его поля.

class Customer:
    def loyalty_discount(self) -> Decimal:
        """Знание о лояльности живёт там же, где данные о лояльности."""
        if self.tier == "gold" and self.years >= 3 and not self.has_debt:
            return Decimal("0.15")
        if self.tier == "silver" and self.years >= 1:
            return Decimal("0.07")
        return Decimal("0")

Цепочки сообщений (Message Chains). order.customer().address().city().name() — вызывающий знает четыре уровня чужой структуры и ломается от изменения любого. Лечение — Hide Delegate, а если делегирующих методов накопилось столько, что класс не делает больше ничего, — обратный рефакторинг Remove Middle Man. Эти два рефакторинга противоположны, и это нормально: почти каждому рефакторингу в каталоге соответствует обратный, потому что оптимум лежит между крайностями и со временем сдвигается.

Неуместная близость (Inappropriate Intimacy). Два класса копаются в приватных потрохах друг друга. В Go/Python/JS, где приватность договорная, этот запах особенно легко заводится: _internal_cache соседнего модуля никто не защищает, кроме дисциплины.

4. Каталог рефакторингов: что применять

Ниже — рабочее подмножество каталога Фаулера (онлайн-каталог), сгруппированное по цели. Это те преобразования, которыми делается 90% реальной работы.

Guard clauses: маленький рефакторинг с большим эффектом

# ДО: «стрела» — глубина вложенности 4, счастливый путь не виден
def payout(employee):
    result = None
    if employee.is_active:
        if not employee.is_separated:
            if employee.has_bank_account:
                result = compute_regular_payout(employee)
            else:
                result = compute_cash_payout(employee)
        else:
            result = compute_final_settlement(employee)
    else:
        result = Money.zero()
    return result

# ПОСЛЕ: особые случаи отстреляны сверху, основной сценарий — в конце, на нулевом отступе
def payout(employee):
    if not employee.is_active:
        return Money.zero()
    if employee.is_separated:
        return compute_final_settlement(employee)
    if not employee.has_bank_account:
        return compute_cash_payout(employee)
    return compute_regular_payout(employee)

Изменение выглядит косметическим, но у него измеримый эффект: цикломатическая сложность та же (4 ветки), а когнитивная сложность падает почти вдвое — метрика Cognitive Complexity от SonarSource штрафует именно вложенность, а не число ветвей. Мозг читателя работает так же: каждый уровень отступа — это ещё одно условие, которое надо удерживать в голове до конца блока.

Split Phase: разделяй расчёт и эффект

# ДО: смешаны чтение конфига, вычисление и запись — не тестируется без БД и файлов
def apply_promo(order_id):
    order = db.fetch_order(order_id)
    rules = json.load(open("/etc/app/promo.json"))
    total = sum(i.price * i.qty for i in order.items)
    for r in rules:
        if r["min_total"] <= total:
            total *= (1 - r["discount"])
    db.update_total(order_id, total)
    mailer.send(order.email, f"Ваша скидка применена, итог: {total}")

# ПОСЛЕ: чистое ядро + тонкая оболочка эффектов
def compute_total(items, rules) -> Money:
    """Чистая функция: те же входы → тот же выход. O(n + m), тестируется без инфраструктуры."""
    total = sum((i.price * i.qty for i in items), Money.zero())
    for rule in sorted(rules, key=lambda r: r.min_total):
        if rule.min_total <= total:
            total = total * (1 - rule.discount)
    return total

def apply_promo(order_id, repo, config, mailer):      # оболочка: только оркестрация
    order = repo.fetch_order(order_id)
    total = compute_total(order.items, config.promo_rules())
    repo.update_total(order_id, total)
    mailer.send(order.email, f"Ваша скидка применена, итог: {total}")

Это тот самый паттерн «functional core, imperative shell», который делает пирамиду тестов дешёвой: compute_total покрывается сотней быстрых unit-тестов и property-based тестами, а apply_promo — парой интеграционных. Подробно — в следующей статье трека, Принципы тестирования.

5. Механика: как рефакторить безопасно

Каталог рефакторингов — это половина дела. Вторая половина — дисциплина исполнения, и именно на ней команды обжигаются.

Пять правил, которые превращают эту схему в работающую практику:

1. Сначала сеть, потом трапеция. Рефакторинг без тестов — это не рефакторинг, а изменение поведения с неизвестным результатом. Если код не покрыт, первым делом пишутся характеризационные тесты (термин Майкла Физерса, Working Effectively with Legacy Code): вы запускаете код, смотрите, что он выдаёт, и записываете это в ассерты — даже если результат кажется неправильным. Задача такого теста не «проверить корректность», а «поймать момент, когда я случайно изменил поведение».

2. Шаг должен быть меньше, чем страшно. Признак того, что вы взяли слишком много: «сейчас не компилируется, но я знаю, что делаю, ещё минут двадцать». Если в этот момент кто-то отвлечёт вас на инцидент, работа потеряна. Классические техники разбиения: Parallel Change (см. ниже) и «сначала добавь новое, потом переключи, потом удали старое».

3. Компилятор — тоже тестовая сеть. В языках с типами (Go, TypeScript, C#, Rust) многие рефакторинги проверяются статически: удалили поле — компилятор перечислит все места. Это причина, по которой рефакторить типизированный код на порядок дешевле, чем нетипизированный. В Python эквивалентом служат аннотации типов и mypy --strict в CI.

4. Автоматический рефакторинг лучше ручного. Rename, Extract Method, Change Signature в IntelliJ/Rider/VS Code/PyCharm выполняются как AST-преобразования и не ошибаются в отличие от «поиск и замена». Правило: если IDE умеет — не делайте это руками. (Обратная сторона: массовое Rename порождает огромные, но тривиальные дифы; такие коммиты полезно делать отдельными и помечать в .git-blame-ignore-revs, чтобы не ломать git blame.)

5. Один коммит — один рефакторинг. Ревьюер должен читать не «800 строк изменений», а «извлечена функция compute_total, вызовы перенаправлены». История, где каждый коммит — атомарное преобразование с зелёными тестами, ещё и позволяет бинарно искать регрессию через git bisect.

Preparatory refactoring: главный приём на практике

Самое ценное правило рефакторинга в проде сформулировал Кент Бек:

For each desired change, make the change easy (warning: this may be hard), then make the easy change. (оригинал в X/Twitter, 2012)

То есть: перед фичей вы не пытаетесь «отрефакторить всё», а делаете ровно то преобразование, после которого ваша фича становится тривиальной вставкой. Фаулер называет это preparatory refactoring и приводит понятную метафору: «это как ехать на восток за 100 км; если сначала проехать 20 км на север к автостраде, доедешь быстрее».

Практический эффект: рефакторинг перестаёт быть отдельной задачей, за которую надо выпрашивать время у менеджера. Он становится частью оценки фичи — «эта фича на 3 дня, из них день на подготовку кода». Это единственный способ рефакторинга, который выживает в командах с продуктовым давлением; «спринт технического долга» не выживает почти никогда, потому что его всегда есть чем вытеснить.

6. Большие преобразования без остановки разработки

Отдельный вопрос — что делать, когда нужно поменять то, что используется в ста местах: формат хранения, интерфейс библиотеки, схему БД. Наивный ответ («ветка refactor/big-one, три недели, потом merge») почти всегда заканчивается адом слияния, потому что trunk всё это время жил своей жизнью.

Parallel Change (expand — migrate — contract)

Приём, описанный Дэнни Диллоном и популяризованный Фаулером:

1. Expand. Добавляем новый интерфейс/поле/таблицу рядом со старым. Никто им не пользуется — изменение безопасно и мгновенно релизится.
2. Migrate. Переводим потребителей по одному, отдельными коммитами. Старая реализация продолжает работать; полезно навесить на неё лог/метрику deprecated_call_total, чтобы видеть остаток.
3. Contract. Когда метрика показала ноль вызовов за разумный период — удаляем старый путь.

Тот же приём в БД называется expand/contract migration: сначала добавить колонку и писать в обе, потом бэкфилл, потом читать из новой, потом удалить старую. Каждый шаг совместим с предыдущей версией приложения — а значит, возможен rolling deploy и откат.

Branch by Abstraction и Strangler Fig

Для замены целой подсистемы вводится абстракция поверх старой реализации, за ней появляется новая, а переключение делается флагом:

Ключевое отличие от долгоживущей ветки: новая реализация попадает в trunk рано и выключенной. Интеграционная боль распределена по мелким коммитам, а не сконцентрирована в конце. См. Branch by Abstraction и Strangler Fig Application — второй паттерн про то же самое на уровне целых сервисов: новый сервис перехватывает запросы по одному эндпоинту, пока старый не станет пустым.

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

Когда рефакторинг — неправильный ответ

Иногда честный ответ — «не рефакторить». Признаки:

• Модуль не меняется и не будет меняться. Проценты по долгу нулевые.
• Код будет удалён через квартал (мигрируем на другой продукт).
• Нет никакого способа проверить поведение: нет тестов, нет спецификации, нет живых людей, кто помнит, как оно должно работать, и нет трафика для shadow run. Тогда сначала обкладываем тестами через характеризацию, и это отдельный проект.

А иногда правильный ответ — переписать. Джоэл Спольски в известном эссе Things You Should Never Do разбирает провал Netscape 6 и формулирует главный аргумент против rewrite: старый код уродлив, потому что содержит тысячи багфиксов — знание, которого нет ни в одной спецификации. Переписывая, вы выбрасываете это знание и будете открывать те же баги заново. Rewrite оправдан, когда меняются нефункциональные требования, которые старая архитектура не выдержит в принципе (другой масштаб, другая модель согласованности, другая платформа), — и даже тогда его делают через Strangler Fig, а не «big bang».

7. Как находить, что рефакторить: метрики и инструменты

Полагаться только на «нос» разработчика не масштабируется: в репозитории на миллион строк запахов больше, чем времени. Нужна приоритизация.

Метрики, которые реально помогают:

• Цикломатическая сложность (McCabe): число независимых путей = число ветвлений + 1. Хорошая тревожная сигнализация: функции с CC > 10 стоит посмотреть, с CC > 20 — почти всегда есть что извлечь. Полезна как фильтр, не как цель.
• Когнитивная сложность (SonarSource): та же идея, но со штрафом за вложенность и за прерывание линейного чтения. Ближе к субъективному «сложно читать».
• Churn — как часто файл менялся за последние N месяцев (git log --format=format: --name-only | sort | uniq -c | sort -rn).
• Fan-in / fan-out — сколько модулей зависит от этого и от скольких зависит он.

Главный приём — пересечение churn и сложности. Идея Адама Торнхилла (Your Code as a Crime Scene, инструмент CodeScene): сложный код, который никто не трогает, вам не мешает; сложный код, который правят каждую неделю, — это то место, где сгорает бюджет команды.

Практика: раз в квартал построить такую карту по своему репозиторию (это 20 строк на Python: git log + radon cc для Python или lizard для мультиязычных проектов) и взять из правого верхнего квадранта две-три цели на следующий квартал. Это превращает разговор с менеджером из «нам не нравится код» в «в этих трёх файлах происходит 40% всех правок и там же 60% инцидентов».

Инструменты, которые стоит включить в CI:

Важное ограничение: ни один инструмент не отличает «сложно, потому что плохо написано» от «сложно, потому что предметная область сложная». Метрика — это фонарик, а не судья. Порог качества в CI (quality gate) стоит ставить на новый код («не хуже, чем сейчас»), а не на весь репозиторий сразу — иначе первая же сборка будет красной, и гейт отключат.

8. Типичные ошибки

1. Рефакторинг без тестов. Самая дорогая ошибка: вы не рефакторите, вы переписываете наугад. Лечится характеризационными тестами.
2. Рефакторинг вперемешку с фичей. Ревью невозможно, git bisect бесполезен, откат фичи тянет за собой откат улучшений.
3. «Большой рефакторинг» как проект на месяц. Не доживает до конца: приоритеты меняются, ветка расходится с trunk. Замена — preparatory refactoring и Parallel Change.
4. Рефакторинг ради метрики. «Разбили функцию на восемь по три строки, CC упала» — читаемость при этом часто падает: восемь прыжков вместо одного связного чтения. Метрика оптимизируется, дизайн деградирует (частный случай закона Гудхарта).
5. Погоня за запахом вместо проблемы. Запах — гипотеза. Прежде чем чинить, ответьте: какое конкретное будущее изменение станет дешевле? Если ответа нет — вы тратите бюджет на эстетику.
6. Преждевременная абстракция под видом рефакторинга. «Здесь три похожих метода, введу базовый класс с шаблонным методом» — а через месяц они разъезжаются, и шаблонный метод обрастает флагами. См. цену преждевременной абстракции.
7. Переименование как единственный вклад в ревью. Массовые переименования в чужом активно правящемся коде порождают конфликты слияния у всей команды. Делайте их отдельным коммитом и предупреждайте.
8. Игнорирование производительности. Replace Temp with Query в горячем цикле может превратить O(n) в O(n²), если запрос считает то же самое заново. Правило Фаулера: рефакторьте ради ясности, а производительность чините потом и по профилировщику, а не по интуиции — но в измеримо горячем коде читаемость иногда приходится обменять на скорость осознанно и с комментарием-обоснованием.
9. Рефакторинг чужого кода без разговора. Если модуль активно правит другая команда, ваш «улучшенный» дизайн придёт к ним в виде конфликтов и сломанных ожиданий.

9. Как это выглядит в проде

Что делают команды, у которых с этим хорошо:

• Boy Scout Rule («оставь стоянку чище, чем нашёл») в мягкой форме: каждый, кто трогает файл, делает одно маленькое улучшение — имя, guard clause, извлечённая функция. Ограничение — «одно», иначе диф разрастается.
• Рефакторинг заложен в оценку. Не отдельный тикет «technical debt», а строка в оценке фичи. Тикеты на долг заводят только для крупных преобразований, требующих координации.
• Правило бюджета. Некоторые команды фиксируют долю ёмкости спринта (10–20%) на техническую работу; это работает, только если доля защищена и не сгорает первой при сдвиге сроков.
• Гейт на новый код. SonarQube/линтеры блокируют деградацию нового кода, а не требуют мгновенно починить легаси.
• Карта горячих точек обновляется ежеквартально и обсуждается на планировании — с цифрами по churn, инцидентам и времени ревью.
• Обязательное правило коммита: сообщение начинается с refactor: (см. Conventional Commits), и такой коммит по определению не меняет поведение. Это позволяет ревьюеру выбрать правильный режим чтения, а релиз-инженеру — понять риск.
• Ретроспектива инцидентов ищет структурные причины. Если постмортем третий раз подряд указывает на один модуль, это не «человеческий фактор», а адрес для рефакторинга.

10. Мини-итог

• Рефакторинг — изменение структуры без изменения наблюдаемого поведения, выполняемое последовательностью маленьких проверяемых шагов. Не смешивайте его с фичами и фиксами.
• Запах — поверхностный признак, часто указывающий на проблему дизайна. Это язык для обсуждения качества, а не список правил. Пять семейств: раздувальщики, нарушители ООП, препятствия изменению, мусор, сцепщики.
• Лечение почти всегда есть в каталоге: Extract Function, Move Function, Introduce Parameter Object, Replace Primitive with Object, Guard Clauses, Split Phase покрывают большую часть повседневной работы.
• У каждого рефакторинга есть обратный. Оптимум — не крайность, а точка, зависящая от того, как система растёт (см. expression problem).
• Безопасность даёт тестовая сеть + микрошаги + компилятор + IDE. Нет покрытия — сначала характеризационные тесты.
• Большое меняется через Parallel Change / Branch by Abstraction / Strangler Fig, а не через долгоживущую ветку.
• Приоритизация — по пересечению частоты изменений и сложности, а не по личному раздражению.
• Главный приём в проде: make the change easy, then make the easy change.

Источники

• Martin Fowler. Refactoring: Improving the Design of Existing Code, 2nd ed. — книга, онлайн-каталог рефакторингов
• Michael Feathers. Working Effectively with Legacy Code — O’Reilly
• Kent Beck, Ward Cunningham и др. — C2 wiki: CodeSmell, WardExplainsDebtMetaphor
• Refactoring.Guru — каталог запахов и рефакторингов
• Martin Fowler. Preparatory Refactoring, ParallelChange, BranchByAbstraction, StranglerFigApplication, OpportunisticRefactoring
• Adam Tornhill. Your Code as a Crime Scene, 2nd ed. — Pragmatic Bookshelf
• SonarSource. Cognitive Complexity: a new way of measuring understandability (PDF, white paper)
• Joel Spolsky. Things You Should Never Do, Part I
• Emerson Murphy-Hill, Chris Parnin, Andrew P. Black. How We Refactor, and How We Know It — эмпирическое исследование: большинство рефакторингов делается вручную, а не через инструменты IDE, и почти не помечается в коммитах

Что дальше

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

Принципы тестирования: пирамида, TDD, BDD, свойства хорошего теста