JavaRush /Курси /Claude code /Інкрементальний рефакторинг legacy-модуля

Інкрементальний рефакторинг legacy-модуля

Claude code
Рівень 27 , Лекція 1
Відкрита

1. Один крок — одна перевірювана гіпотеза

Коли вперше дивишся на старий модуль на кшталт mrr-engine, дуже хочеться обрати один із двох неправильних режимів. «Не чіпаю, воно страшне» консервує проблему. «Claude, перепиши все красиво» створює нову — свіжу, дорогу й із ще злішим diff.

Інкрементальний рефакторинг живе за іншою логікою. Ви берете маленьку ділянку й заздалегідь знаєте, що змінюєте та чим це захищено. Крок вдався — код трохи ясніший. Не вдався — втрачаєте один commit, а не тиждень життя й віру в людство.

Корисно тримати в голові цей цикл:

RISK_MAP → маленька ціль → inspect → межі зміни → edit → checks → diff → review → commit → REFACTOR_LOG

Зверніть увагу на важливу деталь: тут немає кроку «сподіваємося, що Claude сам зрозуміє». Рамку задаєте ви — без неї модель миттєво перетворюється з помічника на енергійного стажера, який заодно вирішив поліпшити ще вісім речей.

Тому сьогоднішня mental model уроку така: один крок — одна перевірювана гіпотеза. Не порятунок системи й не привід «заодно» пофіксити старий баг, оновити dependency та перенести логіку в новий пакет. Пояснюєте за хвилину, що стало краще і чому поведінка не змінилася, — крок обрано правильно.

2. Перший шматок беремо з RISK_MAP.md

До цього моменту у вас уже є RISK_MAP.md з legacy discovery і baseline, зібраний поверх нього в минулій лекції. Отже, ціль обирається не «куди впав погляд», а «що дає користь за мінімального радіуса вибуху». Хороший перший кандидат зазвичай виглядає нудно: винести пошук тарифу в helper, ізолювати логування в маленький метод, витягти одну чисту функцію з довгого методу, прибрати дублювання всередині класу.

Невелика таблиця допомагає швидко відділити корисні цілі від небезпечних:

Кандидат Беремо зараз? Чому
Винести пошук плану в resolvePlan() Так Невеликий радіус, 1–2 файли, поведінка вже прикрита baseline
Переписати весь розрахунок MRR заново Ні Занадто широкий scope, складно довести збереження поведінки
Нормалізувати timezone в усьому модулі Ні Зачіпає багато потоків і легко перетворюється на продуктовий зсув поведінки
Ізолювати запис audit log в окремий метод Так Побічний ефект відокремлюється від розрахунку, diff залишається маленьким
Заодно виправити refund-логіку Ні Це вже bugfix/feature change, а не behavior-preserving refactor

Головне запитання перед вибором кроку звучить так: «Якщо цей refactor виявиться невдалим, чи зможу я відкотити його одним commit і не пояснювати нічого пів дня?» Якщо відповідь «ні», ціль занадто велика.

Ще один корисний критерій — покриття baseline. Якщо обраний шматок уже захищений characterization tests, golden master і хоча б одним smoke-check, це добрий знак. Якщо ж ви не розумієте, які перевірки скажуть вам «поведінка збереглася», змінювати зарано. У такому разі проблема не в нестачі сміливості. Проблема в тому, що в кроку немає safety net.

У CashFlow Dashboard розумний перший крок майже ніколи не лежить у самій нервовій бізнес-логіці. Не варто починати з refund-сценаріїв, складних proration-правил або місць, де одночасно завʼязані billing і payments. Починайте з ділянки, яка вже зрозуміла за RISK_MAP.md, добре читається очима й не тягне за собою три сусідні модулі.

3. inspect-before-edit перед будь-якою правкою

У legacy-контексті особливо небезпечно просити AI «одразу змінити код». Занадто великий шанс, що модель побачить локальну неакуратність і вирішить зробити глобальну красу. Тому тут особливо корисний патерн inspect-before-edit: спочатку Claude читає код і описує мінімальну безпечну трансформацію, і лише потім отримує право на редагування.

Тобто ваш перший запит до Claude в цій ітерації має бути не «рефакторни», а «поясни, що саме ти хочеш відрефакторити і чому». Наприклад, так:

Поки нічого не змінюй.

Подивись лише на клас MrrCalculator і метод calculateMonthlyMrr.
Знайди найменший безпечний refactor без зміни поведінки.

Поверни:
1. що саме пропонуєш змінити;
2. які файли доведеться зачепити;
3. які перевірки з baseline мають залишитися зеленими;
4. які non-goals важливо зафіксувати.

Такий запит добрий одразу з кількох причин. По-перше, він обмежує область уваги моделі. По-друге, змушує її говорити мовою інженерних обмежень, а не мовою «можна зробити красивіше». По-третє, заздалегідь привʼязує майбутній рефакторинг до наявного baseline, а не до абстрактної ідеї якості коду.

Добра відповідь на inspect-запит від Claude зазвичай містить чотири речі. Вона показує точку зміни, пояснює користь, перелічує конкретні файли й привʼязує крок до перевірок. Якщо натомість ви бачите пропозиції на кшталт «заодно спростити кілька сусідніх сервісів», «винести в новий пакет», «перейменувати DTO для однаковості» — це не привід радіти глибині думки. Це сигнал, що scope уже поповз.

Тут корисно бути трохи нудним ревʼюером. Якщо Claude пропонує більше двох файлів на найперший крок, просіть звузити зміну. Якщо зачіпається публічний API, винятки, формат даних або SQL — просіть повернутися на крок назад. Якщо у відповіді немає явного списку перевірок, вважайте, що inspect ще не завершився.

Іноді новачкам здається, що це сповільнює роботу. На практиці все навпаки. Пʼять хвилин на inspect економлять годину на розбір дивного diff, де разом з одним helper-методом раптом приїхали нові назви полів, інший порядок викликів і «невелике поліпшення обробки null». Legacy дуже любить карати за поспіх — і Claude, чесно кажучи, теж.

4. Фіксуємо ціль ітерації та її межі

Після inspect дуже корисно буквально в кількох рядках зафіксувати межі поточної зміни. Це можна зробити в PR_DESCRIPTION.md, у REFACTOR_LOG.md або хоча б у короткій нотатці всередині сесії. Сенс не в бюрократії, а в тому, щоб самому собі не дати розширити задачу дорогою.

Мінімальний шаблон може виглядати так:

## Ціль
Винести пошук плану в helper `resolvePlan()` всередині `MrrCalculator`.

## Не робимо
Не змінюємо сигнатури методів, винятки, SQL, схему БД і розрахунок MRR.

## Перевірки
- characterization: `monthly_mrr_basic`
- golden master: `may24_fixture`
- smoke: розрахунок блоку MRR на dashboard

Зверніть увагу, як працює блок «Не робимо». Він здається простим, але саме він найчастіше рятує рефакторинг від перетворення на «рефактор плюс трішки bugfix плюс маленький redesign, поки код відкритий». Щойно ви явно пишете, що не змінюєте сигнатури, SQL і поведінку винятків, у вас зʼявляється проста причина зупинити Claude: «Це вже поза межами поточної ітерації».

Ще одна корисна звичка — обмежувати розмір зміни не рядками, а здоровим глуздом. Наприклад: один локальний смисловий крок, один маленький diff, максимум один клас і повʼязані з ним тести. Якщо під час правки стає видно, що потрібно зачепити третій файл, краще не героїчно продовжувати, а зупинитися й переоцінити задачу. Іноді це все ще нормальний рефакторинг. Але іноді — вже інший крок, який варто виділити окремо.

У legacy це особливо важливо, бо великий diff обманює. Часто він виглядає «чистим»: код став коротшим, назви красивішими, методи акуратнішими. Але чим більший такий diff, тим складніше довести, що поведінка справді не змінилася. А саме це й є головною умовою сьогоднішньої роботи.

5. Маленький refactor на прикладі mrr-engine

Тепер можна подивитися, як виглядає нормальний перший крок у коді. Припустімо, у MrrCalculator пошук тарифу вбудований прямо в метод розрахунку. Це не катастрофа, але читати й тестувати такий код незручно.

До зміни код може виглядати так:

public BigDecimal calculateMonthlyMrr(Subscription sub) {
    Plan plan = planRepository.findByCode(sub.getPlanCode());
    if (plan == null) {
        throw new IllegalStateException("План не знайдено: " + sub.getPlanCode());
    }
    return plan.getMonthlyPrice(); // поточну поведінку поки не чіпаємо
}

Це ще не жах-жах, але видно, що всередині методу змішані дві думки: знайти план і порахувати MRR. Для першого рефактор-кроку достатньо винести пошук плану в окремий helper, не змінюючи ні сигнатуру, ні виняток, ні підсумковий результат.

Після зміни:

public BigDecimal calculateMonthlyMrr(Subscription sub) {
    return resolvePlan(sub).getMonthlyPrice(); // метод став коротшим і зрозумілішим
}

private Plan resolvePlan(Subscription sub) {
    Plan plan = planRepository.findByCode(sub.getPlanCode());
    if (plan == null) throw new IllegalStateException("План не знайдено: " + sub.getPlanCode());
    return plan;
}

Це добрий крок із кількох причин. По-перше, зовні поведінка залишилася тією ж. По-друге, diff маленький і легко читається. По-третє, у вас зʼявляється локальна точка, навколо якої потім простіше будувати наступні зміни. І, по-четверте, якщо все пішло не так, такий рефакторинг відкочується одним commit без філософських суперечок.

На цьому місці дуже важливо не піддатися спокусі: «раз уже я виніс resolvePlan, давайте ще одразу введемо cache, замінимо виняток, перейменуємо sub на subscription, а заодно нормалізуємо логування». Саме в цей момент маленький корисний крок перетворюється на довгий diff, який уже ніхто не хоче читати до кінця.

Якщо хочеться поліпшити ще щось — чудово. Просто це вже наступний крок. Legacy-модуль не втече. А ось ясність поточної ітерації втече миттєво.

6. Після кроку перевіряємо baseline, а не оптимізм

Найчастіша помилка після вдалого маленького refactor — подивитися на більш красивий код і внутрішньо вирішити: «Ну тут же нічого не зламалося». Legacy особливо любить такі моменти. Бо саме після фрази «та тут же лише helper винесли» зазвичай і зʼясовується, що десь зовні розраховували на текст винятку, порядок викликів або побічний ефект, який ніхто не помітив.

Тому після кожного кроку ви повертаєтеся не до естетики, а до перевірок. Мінімум — до тих, що вже зафіксовані в CHARACTERIZATION_TESTS.md. Наприклад, ось дуже простий characterization test:

@Test
public void shouldKeepMonthlyPriceForBasicPlan() {
    BigDecimal actual = calculator.calculateMonthlyMrr(subscription("BASIC"));
    assertEquals(new BigDecimal("29.99"), actual); // поведінка до і після refactor однакова
}

Сенс такого тесту не в тому, щоб «довести математичну істину». Його завдання — підтвердити, що observable behavior для вже відомого сценарію не зʼїхало через структурну зміну. Саме тому не можна просто переписувати такі тести під нову красу коду. Вони не заважають вам. Вони страхують вас.

Крім автоматичних перевірок, після рефакторингу обовʼязково читають diff. Навіть якщо tests зелені. Навіть якщо Claude впевнено пише «зміни мінімальні». Навіть якщо вам уже хочеться йти далі. У legacy-контексті diff — це візуальний доказ, що ви не потягли за собою зайвого.

Тут дуже доречний reviewer-agent із Workflow Kit. Команда виклику у вашій версії Claude Code може відрізнятися, але сам патерн залишається тим самим: ви даєте незалежному reviewer-контексту поточний diff і просите перевірити саме claims про збереження поведінки.

Наприклад, так:

Перевір поточний diff як refactor-reviewer.

Потрібно відповісти:
- чи збережено поведінку за заявленими сценаріями;
- чи дотримано non-goals;
- чи немає зайвих файлів і прихованих змін;
- які перевірки обовʼязково мають бути зеленими.

Код не змінюй.

Якщо reviewer-agent пише, що diff зачіпає більше, ніж обіцяно, або бачить приховану behavior change, не сперечайтеся з ним у стилі «але intention була добра». Тут intention вторинна. Важливий лише результат у коді та перевірках. А якщо baseline раптом почервонів, не треба просити Claude «трохи підправити тест». Спочатку відкотуйте останній крок, перечитайте diff і поверніться до inspect. Тести — це safety net, а не перешкода на шляху до краси.

Хмарне review

Окрім локального reviewer-agent у цій роботі доречний ще один шар — хмарне ревʼю (умовно /ultrareview або аналог — точна назва й доступність можуть змінюватися від версії до версії). Legacy-рефакторинг особливо схильний до прихованих регресій: поведінка змінюється непомітно, тому що характеризаційні тести, які ми готували в минулій темі, можуть не покривати всі критичні шляхи — у legacy-коді покриття історично слабке.

Хмарне ревʼю тут працює як додатковий детектор регресій на кожен refactor PR: кілька агентів розбирають diff із фокусом на збереження поведінки, побічні ефекти, приховані припущення та сумісність із залежними модулями. Завдяки більшому контексту хмарна поверхня ловить звʼязки між модулями, які локальний reviewer subagent пропускає.

Застосування просте: запускаєте хмарне ревʼю після того, як характеризаційний suite пройшов локально і refactor PR готовий до розбору. Розмічений diff із рівнями серйозності доповнює вашу детерміновану страхувальну сітку на семантичному рівні — але не замінює її. Тести доводять, що поведінка не поїхала; хмарне ревʼю лише підсвічує ризиковані патерни. Final approval усе одно залишається за людиною.

І є три випадки, коли хмарне ревʼю для legacy не варто застосовувати взагалі: код, який не можна вивантажувати в хмару через чутливі дані або вимоги on-prem; зовсім маленький рефакторинг одного методу, де витрати на запуск хмарного ревʼю більші за користь; і слабкий характеризаційний suite — доки критичні шляхи не покриті хоча б наполовину, спочатку зміцнюйте страхувальну сітку, а не женіть хмарне ревʼю поверх порожнечі. Плагін допомагає, але не затверджує — це правило однаково працює і для локального reviewer-agent, і для хмарної поверхні.

7. Commit і REFACTOR_LOG.md тримають зміст ланцюжка

Legacy-модернізація рідко ламається на одному великому рішенні. Набагато частіше вона ламається на десятому маленькому кроці, бо вже ніхто не памʼятає, навіщо робили третій і що саме захищав сьомий. Тому після кожного вдалого кроку рефакторингу корисно не лише закомітити код, а й залишити короткий слід змісту.

Дуже зручний для цього REFACTOR_LOG.md. У цій модульній лінії він працює як єдиний робочий журнал: спочатку ви фіксуєте в ньому маленькі кроки рефакторингу, потім candidate slices і parity-нотатки, а далі — фази roadmap, щоб історія модернізації не розповзалася по різних файлах. Він може бути буквально на кілька рядків:

## Крок 1 — `resolvePlan`
Ціль: винести пошук плану з `calculateMonthlyMrr`
Змінені файли: `MrrCalculator.java`, `MrrCalculatorTest.java`
Перевірки: `monthly_mrr_basic`, `may24_fixture`
Результат: зелені
Наступний кандидат: відокремити audit log від розрахунку

Такий лог розвʼязує одразу кілька задач. По-перше, зберігає контекст між сесіями Claude. По-друге, допомагає вам самому не забути, чому поточна форма коду взагалі зʼявилася. По-третє, якщо до модуля приєднається інша людина або інший агент, йому не доведеться гадати, де тут уже був усвідомлений крок, а де просто випадкова форма legacy.

Commit при цьому теж має бути вузьким і чесним. Не cleanup, не fix stuff, не improve code. Краще щось на кшталт: refactor(mrr): винести resolvePlan із calculateMonthlyMrr. Так, звучить менш героїчно. Зате через два тижні ви самі собі скажете спасибі.

На цьому місці корисно розрізняти три рівні роботи. Якщо зміна досі живе всередині одного класу або контракту і baseline її закриває, залишаємося в local refactor. Якщо кожен наступний крок впирається в SQL, audit, legacy-format або інший змішаний dependency-комок, час шукати seam і розрізати звʼязки. А якщо старий і новий шлях мають якийсь час жити паралельно під флагом і порівнюватися на parity, це вже Strangler-сценарій, а не просто ще один helper-method.

Саме з таких маленьких і добре зафіксованих кроків потім складається справжня модернізація. Не з надихаючої ночі, коли «ми нарешті переписали це стародавнє чудовисько», а з ланцюжка нудних, зелених змін, які можна нормально перевірити на review, після яких mrr-engine перестає бути чорною скринькою і починає виглядати як серія зрозумілих рішень.

Коментарі
ЩОБ ПОДИВИТИСЯ ВСІ КОМЕНТАРІ АБО ЗАЛИШИТИ КОМЕНТАР,
ПЕРЕЙДІТЬ В ПОВНУ ВЕРСІЮ