Токсичный рефакторинг: когда хорошие намерения приводят к проблемам

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

Напомню, что такое рефакторинг. Это процесс улучшения кода без изменения его поведения. Часто приводят аналогию с уборкой в комнате: мы убираем вещи, приводим в порядок пространство, но сама комната остается той же – четыре стены, двери, окна.

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

Признаки качественного рефакторинга

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

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

Третий признак – сохранение исходного поведения кода. Код до преобразования и после него должен вести себя одинаково. Ничего кардинально меняться не должно.

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

Когда к качеству рефакторинга есть вопросы

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

Рефакторинг очень старого кода

Первый пример – рефакторинг очень старого кода, legacy-кода. Какие здесь возникают проблемы:

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

• Большие затраты по времени и ресурсам. Если нет планов дальше развивать этот legacy-код, лучше его не трогать. Такие усилия будут пустыми.

• Важно помнить, что любое изменение старого кода может нарушить баланс, достигнутый за годы промышленной эксплуатации. Здесь хорошо подходит философия «работает – не трогай».

Предрелизный рефакторинг

Второй пример – предрелизный рефакторинг.

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

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

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

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

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

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

Здесь важно помнить принцип YAGNI («You aren't gonna need it» – «Вам это не понадобится»). В ситуации рефакторинга ради рефакторинга код усложняется, а принцип нарушается. По сути, время тратится впустую.

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

Как избежать ловушки перфекционизма?

• Важно понимать, что цель – улучшить код, но не стремиться к идеалу во что бы то ни стало. Это особенно важно на больших проектах. Доводить код до идеального состояния сложно, избыточно и дорого.

• Необходимо понимать, какое решение является плохим, какое – приемлемым, а какое – идеальным. Оптимальный вариант – приемлемый код, в котором разработчик хорошо ориентируется, понимает, где может возникнуть ошибка, и в случае необходимости быстро вносит правки. Исправил и система снова работает корректно. Идеальный код – это тоже хорошо. Как говорит Линус Торвальдс: «Это хороший вкус к программированию», он элегантен, минималистичен и в нем нет ничего лишнего. Однако доведение кодовой базы до такого состояния – долгий процесс, и не всегда им стоит заниматься.

• Вместо вопроса «что я могу улучшить еще?» полезнее задать вопрос «Могу ли я остановиться в своих рефакторинговых изысканиях?». Умение вовремя остановиться – важное профессиональное качество. В этом помогают code-review и соблюдение принципа KISS («Keep it simple, stupid» – «Делай проще, тупица»).

Рефакторинг без тестирования

Рефакторинг без тестирования означает внесение изменений без какой-либо проверки.

• Основной риск в этом случае – привнесение в код скрытых ошибок. При отсутствии тестов остается надеяться на случай.

• Существует риск потери части функциональности: можно случайно изменить или удалить важную бизнес-логику.

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

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

Рефакторинг про запас

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

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

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

• Вместо решения реальных проблем время тратится на оптимизацию гипотетических сценариев.

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

Выделение функциональности в универсальный мегаметод

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

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

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

• Возникает высокая цикломатическая и когнитивная сложность из-за избытка условий.

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

Какие подходы можно использовать:

• Разделение ответственности. Общую функциональность лучше выделять в небольшие методы.

• Каждый метод должен выполнять только одну конкретную задачу.

• Небольшое дублирование кода иногда лучше, чем преждевременная или избыточная абстракция. Здесь важно правильно понимать принцип DRY («Don’t Repeat Yourself» – «Не повторяйся»). Его не стоит воспринимать буквально. Главное – не допускать дублирования бизнес-логики. Повторяющийся синтаксис в отдельных случаях допустим.

Рефакторинг программного интерфейса

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

Какие основные проблемы могут возникнуть:

• Первая и самая важная – нарушение обратной совместимости. Если в методе программного интерфейса что-то меняется, например он переименовывается, изменяется сигнатура или тип возвращаемого значения, это может привести к нарушению обратной совместимости.

• Вторая проблема – несоответствие документации и описания контракта программного интерфейса. Документация может устаревать и переставать отражать реальное поведение кода, поскольку разработчики не всегда своевременно вносят в нее изменения.

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

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

Рефакторинг без понимания бизнес-логики

Рефакторинг без понимания бизнес-логики приводит к ряду проблем:

• Нарушение критических бизнес-правил. Код может выглядеть как плохой, но на практике в нем могут быть заложены особые условия: специальные скидки для клиентов, специфические правила округления, например с точностью до 18–20 знаков после запятой.

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

• Усложнение поддержки. Со временем реальный код может разойтись с техническим заданием и бизнес-процессами, которые описаны в документации.

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

Пример ошибки при рефакторинге без понимания бизнес-логики: до рефакторинга ключ добавлялся в структуру по условию, после рефакторинга ключ добавляется при инициализации. Далее в коде возникает ошибка, так как выполняется проверка на существование ключа в этой структуре.

Отдельные задачи под рефакторинг

Отдельные задачи под рефакторинг – это вопрос накопления технического долга. Рассмотрим возникающие проблемы:

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

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

• Накапливание технического долга. Увеличивается объем потенциальной работы для команды. Это происходит потому, что новый код пишется поверх старого. Старый код остается неотрефакторенным, а новый добавляется поверх него. В результате объем потенциальной работы возрастает.

Как можно избежать этих проблем:

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

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

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

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

Отсутствие единого стандарта рефакторинга

Отсутствие единого стандарта рефакторинга – ситуация «лебедь, рак и щука». Каждый разработчик действует по-своему, исходя из собственного опыта и насмотренности.

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

• Проблемы при проведении код-ревью. Ревьюер начинает уделять больше внимания стилистическим особенностям кода, чем изменениям функциональности.

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

В качестве решения можно предложить создание внутренних стандартов. Их можно разрабатывать самостоятельно, но разумно опираться на стандарты разработки фирмы 1С. В таких стандартах стоит зафиксировать правила именования и организации кода. Полезно включить примеры хорошего и плохого рефакторинга с точки зрения команды.

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

Также важно использование современных инструментов анализа качества кода, таких как АПК, SonarQube, BSL LS, Phoenix BSL.

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

ВТ_Остатки
втОстатки
ВтОстатки
ВрТабОстатки
Остатки

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

Рефакторинг несущественных деталей задачи

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

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

Что в результате:

• Происходит затягивание сроков, срыв дедлайнов.

• Команда тратит больше времени и усилий, чем планировалось.

• Потеря фокуса. Мелкие детали отвлекают от более важных задач.

• Привнесение новых ошибок.

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

Приведу два примера из практики.

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

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

Так когда же рефакторить?

После рассмотрения большого количества примеров может показаться, что рефакторить вообще не стоит. Однако это не так.

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

• Рефакторинг уместен и при исправлении ошибок, так как ошибки часто являются следствием сложного и запутанного кода.

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

• Также стоит помнить о правиле трех (Rule of Three). Если один и тот же код встречается три раза и возникает желание его отрефакторить, то именно на третий раз это действительно стоит сделать.

Рефакторить или нет?

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

«Рефакторинг должен быть таким же привычным, как чистка зубов. Но если делать это топором, последствия будут болезненными» (М. Фаулер).

*************

Статья написана по итогам доклада (видео), прочитанного на конференции INFOSTART TEAMLEAD&CIO EVENT.