Поток лавы
Мой любимый антишаблон. Начинается всё просто: в новый проект можно добавить код из старого проекта копипастой. Он там делал что-то полезное — пускай делает почти то же самое и в новом проекте. Вот только нужно закомментировать один кусок, а в другом месте — чуть дописать.
Примерно через три переноса без рефакторинга образуются большие закомментированные участки, функции, которые работают только с частью параметров, сложные обходы вроде «выльем воду из чайника, выключим газ — и это приведёт нас к уже известной задаче кипячения чайника», и так далее.
Обратите внимание: паттерн подкупающе хорош в кратковременной перспективе и становится плох только на дальней дистанции. И смертельно плох на очень дальней.
Симптомы
- Непонятно откуда взявшиеся переменные.
- Не относящиеся к задаче фрагменты кода.
- Очень странное покрытие документацией, много выглядящего важным за её пределами.
- Архитектура слеплена из предыдущих трёх архитектур.
- «Да кто её будет смотреть, просто подключайте к проекту!»
- Устаревшие интерфейсы.
Почему возникает
Это если команда одна. А если разработчики на пятом проекте новые, то начинается самое весёлое — этот сталактит надо ещё прочитать.
Очень часто я вижу лава-код в проектах аутсорсинговых компаний, потому что они используют свою кодовую базу по разным заказчикам как своеобразный «иннерсорс». А «междисциплинарный» код как раз хорошо обрастает отключаемыми участками и переопределяемыми функциями.
В API-driven-подходе и Domain-Driven Development такого в принципе не должно случаться, а мы хотим думать, что придерживаемся этих идей. Точнее, архитекторы уверены, что придерживаемся, а дальше уже как пойдёт в конкретных командах.
Пример
«Плохой» вариант: функция, перенесённая из старого проекта, с мёртвыми параметрами, закомментированным кодом и обходным путём:
def process_payment(payment, account, legacy_flag, retry=False, old_currency=None):
# legacy: раньше здесь была синхронизация с mainframe
# if legacy_flag:
# sync_mainframe(payment)
# wait_for_response()
# apply_compensation(payment)
if retry and not payment.status == "pending":
return None
if old_currency is not None:
payment.amount = convert_currency(payment.amount, old_currency, "RUB")
# обход: когда-то balance бывал None из-за миграции
balance = account.balance or {}
account.balance = balance
account.balance["amount"] = account.balance.get("amount", 0) - payment.amount
return account
«Хороший» вариант: только то, что относится к задаче, с явными параметрами:
def process_payment(payment: Payment, account: Account) -> Account:
if payment.status != "pending":
raise PaymentNotPending(payment.id)
account.withdraw(payment.amount)
payment.mark_processed()
return account
Как исправить
- Вырезать мёртвый и закомментированный код целиком — он есть в истории версий.
- Перевести код на явные интерфейсы и убрать флаги-рудименты.
- Документировать только то, что действительно работает и используется.
- Если рефакторинг слишком дорог — не переносить ветку дальше, а разбирать по кускам на основе поведения (характеризационные тесты).
Если к вам подходит более опытный разработчик и говорит: «Ты так не делай, сейчас расскажу, как обойти, эта функция вообще опасная, с ней лишний раз не связывайся» — это как раз оно. С такими местами возможна прогулка по минному полю: вы не знаете, какой фрагмент кода как точно работает, и некоторые очевидные решения приводят к интересным для отладки последствиям.