10 распространённых запахов кода и как от них избавиться

Введение

Термин «запах кода» (code smell) описывает фрагмент кода или шаблон программирования, который сам по себе может не содержать багов, но указывает на системную проблему в дизайне. Запахи облегчают идентификацию мест, где стоит провести рефакторинг — уменьшить технический долг, повысить расширяемость и упростить сопровождение.

Ниже — 10 наиболее распространённых запахов кода с практическими рекомендациями, примерами, когда не стоит рефакторить, плюс дополнительные материалы: чек-листы, плейбук, decision tree и краткий глоссарий.

1. Сильная связанность

Описание

Сильная связанность (tight coupling) — когда два класса/модуля зависят друг от друга настолько, что изменение одного требует правок в другом. Это усложняет изменения и повышает риск регрессий.

Пример проблемы

class Worker {
  Bike bike = new Bike();
  public void commute() {
    bike.drive();
  }
}

Почему это плохо

• Трудно заменить зависимость (например, заменить Bike на Car).
• Снижается переиспользуемость и тестируемость.

Решение

Ввести абстракцию — интерфейс или базовый класс, чтобы Worker зависел от «Vehicle», а не от конкретной реализации:

class Worker {
  Vehicle vehicle;
  public void changeVehicle(Vehicle v) {
    vehicle = v;
  }
  public void commute() {
    vehicle.drive();
  }
}

interface Vehicle {
  void drive();
}

class Bike implements Vehicle {
  public void drive() {}
}

class Car implements Vehicle {
  public void drive() {}
}

Альтернативные подходы

• Внедрение зависимостей (DI) через конструктор или фабрику.
• Использовать события/сообщения вместо прямых вызовов (event-driven).

Когда не рефакторить

• Маленький экспериментальный прототип, который будет выкинут.
• Жёсткие временные ограничения на выпуск, когда рефакторинг создаёт риск багов.

Критерии приёмки

• Класс Worker больше не создаёт конкретную реализацию Vehicle.
• Можно подставить моки в тестах без изменений Worker.

2. Бог-объекты

Описание

God object — класс или модуль, в который свалено слишком много данных и функций. Он «знает слишком много» и «делает слишком многое».

Почему это плохо

• Увеличивает связанность; другие части системы опираются на один большой объект.
• Трудно понять и тестировать.

Как исправить

Разделите ответственность: выделите логические группы данных и поведения в отдельные сущности.

Пример

class User {
  public String username;
  public String password;
  public String address;
  public String zipcode;
  public int age;
  ...
  public String getUsername() { return username; }
  public void setUsername(String u) { username = u; }
}

Лучше:

class User {
  Credentials credentials;
  Profile profile;
  ...
}

class Credentials {
  public String username;
  public String password;
  public String getUsername() { return username; }
  public void setUsername(String u) { username = u; }
}

Рекомендации

• Применяйте принцип единственной ответственности (SRP).
• Используйте композицию вместо монолитных классов.

Когда не рефакторить

• Если класс достаточно стабилен, изменения часты и рефакторинг даст мало преимущества.

3. Длинные функции

Описание

Функция разрослась и выполняет несколько задач. Часто её сложнее читать и тестировать.

Решение

Разбить на мелкие функции, каждая из которых делает одну вещь. Оригинальная функция превращается в последовательность вызовов вспомогательных методов.

Практическая подсказка

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

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

• Изначально: processOrder() — валидирует, вычисляет скидки, делает платёж и отправляет уведомления.
• После: validateOrder(), calculateDiscount(), processPayment(), sendNotifications().

4. Избыточные параметры

Описание

Метод или конструктор принимает слишком много аргументов. Это усложняет чтение, тестирование и понимание ответственности функции.

Рекомендации

• Порог: будьте осторожны при более чем 3 параметрах.
• Объединяйте параметры в объекты настроек или value objects.
• Разделяйте функцию на несколько с более узкой ответственностью.

Пример

Вместо longMethod(a, b, c, d, e) используйте объект контекста: longMethod(Config cfg) или разбейте на несколько шагов.

5. Плохо подобранные имена

Описание

Ненаглядные имена переменных, функций и классов делают код непонятным. Однобуквенные имена, суффиксы с типами и смешение стилей — частые ошибки.

Рекомендации

• Переменные должны быть короткими, но описательными.
• Функции должны содержать глагол и объяснять действие.
• Соблюдайте соглашение по именованию во всём проекте.

Практические приёмы

• Во время ревью требуйте переименований, если имя сбивает с толку.
• Используйте рефакторинг IDE для безопасного переименования.

6. Магические числа

Описание

Хардкодированные константы в коде без объяснения, что они означают. Через несколько месяцев никто не поймёт, почему 42 используется в расчёте.

Решение

• Заменять магические числа на константы с говорящими именами.
• Для наборов значений использовать перечисления (enum).

Пример

Вместо if (status == 3) лучше if (status == STATUS_APPROVED).

7. Глубокая вложенность

Описание

Многократные вложенные циклы и условные операторы усложняют чтение и понимание логики.

Почему это появляется

• Попытка обрабатывать вложенные структуры данных «в лоб».
• Слишком много логики в одном методе.

Решения

• Вынесите логику в методы уровня сущности — запросы и аггрегаторы данных.
• Для большого количества условностей рассмотрите паттерны State или Strategy.
• Для многомерных наборов данных используйте функциональные методы (map/reduce) или stream API.

Когда глубокая вложенность допустима

• Алгоритмы с естественной вложенностью (например, трёхмерная математика) бывают неизбежны.

8. Непойманные или неправильно обработанные исключения

Описание

Игнорирование ошибок, перехват Exception без логирования или повторного бросания создает ситуации, когда приложение «падает молча».

Рекомендации

• Логируйте стек вызовов при перехвате исключений.
• Ловите конкретные типы исключений, а не общий Exception.
• При необходимости делайте повторные попытки (retry) или возвращайте понятную ошибку пользователю.

Пример хорошей практики

try {
  operation();
} catch (SpecificException e) {
  logger.error("Ошибка при операции: {}", e.getMessage(), e);
  throw new BusinessException("Не удалось выполнить операцию");
}

9. Дублирование кода

Описание

Одни и те же фрагменты логики повторяются в разных местах. Изменение требует правок во всех копиях — легко пропустить одно место.

Решение

• Выносите повторяющийся код в общие функции/утилиты.
• Для повторяющегося поведения используйте один сервис или библиотеку.

Пример

Исходный цикл для проверки онлайн-статуса пользователя вынесите в функцию isUserOnline(queryUsername).

10. Отсутствие комментариев

Описание

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

Рекомендации

• Комментарии должны объяснять «почему», а не «что делает» код.
• API и публичные функции должны иметь блоки документации с примерами использования.
• Не комментируйте очевидное; объясняйте нестандартные решения и ограничения.

Общие принципы предотвращения запахов кода

• Придерживайтесь DRY и SRP.
• Пишите тесты — тесты помогают безопасно рефакторить.
• Проводите код-ревью с фокусом на читабельность и архитектуру, не только на баги.
• Инструментируйте код статическим анализом и правилами форматирования.

Важно

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

Дополнительная ценность для команд

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

Быстрая методология рефакторинга (мини-SOP)

1. Поймите цель кода. Прочитайте тесты или напишите их, если их нет.
2. Сделайте небольшие безопасные рефакторы (переименование, вынесение методов).
3. Запустите тесты и проверьте сборку.
4. Публикуйте изменения отдельно от функциональных правок (отдельный PR).
5. Добавьте документацию и обновите чек-лист ревью.

Чек-лист для ревью кода (роли)

• Junior Developer

  • Понимаю ли я, что делает функция?
  • Есть ли у переменных понятные имена?
  • Нужны ли комментарии?

• Senior Developer

  • Есть ли признаки сильной связанности или God object?
  • Соответствует ли код архитектурным принципам?
  • Можно ли улучшить тестируемость без изменения поведения?

• Team Lead

  • Уменьшился ли технический долг?
  • Оптимально ли распределены ответственности между сервисами?

Шаблон задач на рефакторинг (issue)

• Описание запаха: кратко.
• Место в кодовой базе: файлы/модули.
• Предложенное решение: шаги.
• Риски: описание возможных регрессий.
• Критерии приёмки: тесты, метрики, отсутствие побочных эффектов.

Decision tree для оценки «рефакторить или нет» (Mermaid)

flowchart TD
  A[Наблюдается запах кода?] --> B{Влияет на релиз \nили критичен для функционала?}
  B -->|Да| C[Создать отдельную задачу на рефакторинг]
  B -->|Нет| D{Есть ли тесты для кода?}
  D -->|Нет| E[Написать тесты сначала]
  D -->|Да| F[Рефакторить в рамках небольших изменений]
  C --> G[Планировать в спринт]
  E --> G
  F --> H[Отправить PR с тестами]

Риски и их смягчение

• Риск: регрессии после рефакторинга.
  Смягчение: полное покрытие тестами, мелкие изменения, feature-branch и canary rollout.

• Риск: неправильная интерпретация требований.
  Смягчение: обсудить с автором/владельцем фичи, добавить тесты, которые фиксируют текущее поведение.

Критерии приёмки

• Все юнит-тесты и интеграционные тесты проходят.
• Функциональные сценарии, описанные в таске, валидируются.
• Никаких нарушений интерфейсов (backward compatibility) без согласования.

Короткий глоссарий

• Запах кода — признак потенциальной проблемы в дизайне кода.
• SRP — принцип единственной ответственности.
• DRY — не повторяй себя.
• DI — внедрение зависимостей.

Когда запах не критичен

• В prototypes и POC, где важна скорость проверки идеи.
• В тестовых скриптах и утилитах, живущих недолго.
• Там, где изменения дороже пользы (legacy с высоким риском).

Заметьте: решение «не рефакторить» должно быть явным и документированным, а не отговоркой.

Шпаргалка для быстрого рефакторинга

• Переименование переменных: улучшает читаемость без изменения логики.
• Вынести повторяющуюся логику в функцию: уменьшает дублирование.
• Ввести интерфейс/абстракцию: уменьшает связанность.
• Объединить параметры в объект: упрощает сигнатуры.

Заключение

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

Источник изображения: SIphotography/ Depositphotos