SavePearlHarbor

Скучный Рефакторинг: борьба с искушениями

—

от автора

admin

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

Стратегия: копировать проверенную логику as-is, новая обвязка вокруг старого кода и feature flag для отката. Ниже мои принципы для такого рода задач.

Что вы узнаете

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

  2. Почему успех в таких задачах — это сохранённое поведение именно вместе с известными багами и недокументированным поведением.

  3. Как тесты помогают выстраивать четкую границу и соблюдать ее при переносе

  4. Зачем при выносе модуля держать в конфиге переключатель между старым и новым вариантом.

1. “Благими намерениями вымощена дорога в ад”

Самый частый сценарий. Ты открыл файл. Увидел сервис на пятьсот строк. Рука сама тянется сделать нормально. Ещё обновить библиотеку. Ещё вынести общий util. Ещё поправить вот этот странный if, раз уж мы здесь.

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

В любой системе, которая живёт в проде, есть неявный контракт. Клиенты, очереди, тайминги, костыли под конкретный чужой API. Он не лежит в README. Он размазан по логам, коммитам и головах тех, кто помнит инцидент три года назад. Задача рефакторинга не в том, чтобы сделать красиво, а в том, чтобы сохранить этот контракт, меняя только форму. Если за один заход ты тащишь слишком много изменений, ты уже не рефакторишь. Ты выкатываешь релиз с неизвестным количеством сюрпризов.

2. Один тип изменения за раз

Я это формулирую так: сначала границы, потом внутренняя красота.

  • Задача А: вынести модуль, подключить тот же контракт на границе (шина, HTTP, очередь, неважно что), переключатель для отката.

  • Задача Б: переписать стиль, фреймворк, библиотеку.

  • Задача В: починить накопившиеся баги.

  • Задача Г: ускорить и упростить.

Смешать А+Б+В в одной ветке: верный способ получить долгое ревью, неясный rollback и спор «это регрессия или фича».

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

Хуже того, в команде не все понимают зачем так осторожно действовать. И если среди программистов понимание в основном есть, то те кто ближе к бизнесу часто воспринимают это как лишнюю трату времени. Им нужно объяснять и пояснять. И не жалеть на “бесполезные объяснения” время, они полезные.

3. Feature flag

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

Фича флаг не просто “галочка”, это возможность:

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

  • раскатить новый релиз на стенд и на прод не одним махом, а по шагам и частями

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

Идеальный вариант: переключение без редеплоя (конфиг, env и тд). Тогда инцидент ночью превращается в смену флага, а не в экстренный созвон в пол третьего ночи.

4. «Копируй поведение», не «придумывай заново»

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

Упрощённый пример. Было: маршрутизация, размазанная по актору. Стало: чистая функция, которую можно вызвать из теста без ActorSystem:

object UpdateRouting {  def route(update: Update): Either[RoutingError, RouteTarget] =    if (update.message.exists(_.chat.isGroupOrSupergroup)) Right(GroupFlow)    else if (update.callbackQuery.isDefined) Right(CallbackFlow)    else Right(PrivateFlow)}

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

def toBusMessage(in: PlatformInMessage): BusIncoming =  BusIncoming(    correlationId = newCorrelationId(),    channelId = in.channelId.getOrElse(""),    userId = in.channelUserId.getOrElse(""),    text = in.text.getOrElse(""),    occurredAt = Instant.ofEpochMilli(in.ts)  )

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

5. Чего я сознательно не делал

Пока старый код еще жив и есть feature flag точно надо откладывать:

  • Новые библиотеки «потому что свежая». Каждая добавляет новые транзитивные зависимости, новую поверхность атаки, если говорить про ИБ, новые лицензии и новую головную боль в момент, когда что-то внезапно упадёт в проде. Задача на вынос не лучший момент обновлять зависимости или «попробовать новую штуку».

  • Новый стек или стиль на всём модуле. Пока цель совпасть с поведением старого, смесь стилей (старый домен + новая обвязка) нормальная плата за предсказуемость. Выглядит уродливо, зато понятно, что откуда.

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

  • Оптимизации «раз уж открыли файл». Отдельное измерение, отдельный профиль нагрузки, отдельное решение. Рефакторинг и оптимизация, два разных разговора про одни и те же строчки кода.

Звучит занудно. Зато через полгода, когда кто-то будет смотреть git blame, он увидит понятную историю: «вынесли модуль, подключили шину», а не «вынесли модуль, переехали на новый фреймворк, попутно починили три бага и переименовали половину классов».

6. Тесты — это основа

Unit-тесты на чистые функции (маршрутизация, конвертация моделей). Это не покрытие ради KPI. Это фиксация поведения, которое ты боишься потерять при переносе. Написал до переноса, получил контракт. После переноса зелёный тест значит «поведение то же самое».

E2E-тесты нужны там, где unit не справляется: сеть, контейнеры, очередь, база. Но и они не панацея. Если мок внешней системы написан с теми же предположениями, что и код, получишь согласованное враньё. В CI всё зелёное, на проде — инцидент.

У меня был такой случай. Сервис дёргал чужой API за файлами и строил URL так, как казалось логичным. Тестовый мок отдавал файлы по тому же логичному пути. Зелёный e2e. А настоящий API был с префиксом /api/, о чём тест не знал и знать не мог, потому что сам же придумал контракт. На стенде честный 404, разбираться пришлось вручную.

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

7. Когда кода стало больше, это нормально

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

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

Итого

Надёжный рефакторинг legacy для меня это:

  1. Четкая цель. Переносим поведение “as is”, а не «всё сразу».

  2. Предсказуемый дифф. Через полгода понятно, что меняли и зачем.

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

  4. Переключатель в конфиге: новый код не зашёл, отключил и вернул старый. Без этого откат дороже, чем сам перенос.

  5. Тесты как основа: сначала фиксируешь поведение тестами, потом переносишь код.

Альпинисты бывают либо старые, либо смелые. Рефакторинг это про “старых альпинистов”: меньше сюрпризов для команды и меньше разговоров в саппорте в духе «это баг после рефакторинга или так задумано».

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

Удачных вам релизов !

ссылка на оригинал статьи https://habr.com/ru/articles/1025232/