Ошибки во время рефакторинга могут дорого обойтись. Модернизация, ведущая к отказу системы, или внесение новой функциональности параллельно с ошибочными правками явно принесут вред. Но степень вреда может быть разной.
▍ Любой рефакторинг сопряжён с рисками
Доработка кода – это рисковая затея, поскольку подразумевает изменение работающей системы. Всё верно. Но во многих ситуациях разработчики могут снизить эти риски, сделав их допустимыми.
Некоторые задачи рефакторинга подразумевают крупные изменения и затрагивают несколько подсистем. Другие при этом ограничиваются одним компонентом, но могут непредвиденно повлиять на другие части системы и вызвать поломку важнейших бизнес-операций. В этом случае речь может идти о действующем потоке приобретения товара. Третья категория – это доработки, позволяющие внести новые возможности – например, изменение потока приобретения одного товара для поддержки большего числа его единиц и добавление ещё одного потока после.
Объединяет все эти сценарии то, что они сопряжены с высоким риском.
- В случае ошибки такие доработки навредят бизнесу (потеря прибыли, недовольство клиентов), команде (подорвут доверие, мотивацию) или другим связанным функциям (разработка встанет).
- Причём реализация этих изменений весьма затратна, так как требует повышенного внимания, усилий и времени. Предпочтительнее для таких задач задействовать опытных разработчиков, хорошо разбирающихся в этой области.
▍ Как снизить риски
Рекомендую использовать чек-лист:
- Определите ограничения. Как далеко можно зайти?
- Изолируйте доработки от функциональности. Не применяйте их вместе.
- Напишите обширные тесты, более высокоуровневые (интеграционные) с меньшим числом деталей реализации, и сопровождайте ими внесение изменений.
- Проверьте всё наглядно. Запустите браузер.
- Не пропускайте тесты. Не ленитесь.
- Не полагайтесь слишком сильно на код-ревью и контроль качества (QA). Люди ошибаются.
- Не смешивайте масштабные зачистки с другими изменениями. Это можно делать в случае небольших доработок.
Может возникнуть соблазн просто всё переписать, но…
▍ Не спешите
1. Заранее оцените, насколько рисковым (затратным) окажется изменение с позиции разработки и бизнеса.
Что произойдёт, если некачественный код попадёт в продакшен? Нарушение бизнес-целей, недовольство пользователей или утрата доверия и прочее – всё это является серьёзными проблемами. Поэтому продумывание возможных негативных последствий будет хорошим первым шагом. Может, пока вообще не стоит браться за рефакторинг?
2. Подумайте, будет ли доработка реализовываться как отдельная задача или же в рамках внесения новой функциональности.
Реализация новой функции создаёт естественные условия для рефакторинга, который пойдёт на пользу ей самой. В таком случае практичным будет сделать это сразу, пока вы находитесь в соответствующем контексте. «Потом» обычно никогда не наступает.
Но при этом желательно отделить уборку от самой разработки новой функции, чтобы эти процессы прошли отдельные ревью, особенно когда ожидается, что улучшение внесёт побочные эффекты, или его будет сложно проверить.
3. Предварительно убедитесь в работоспособности системы.
Прежде чем начинать какую-либо работу, важно проверить, чтобы все связанные с ней части были функциональны. Причём прохождения всех тестов приложения будет недостаточно. Эти тесты охватывают конкретные пользовательские потоки, но могут упускать другие. Так что здесь важно восполнить все пробелы необходимыми тестами.
▍ Такой рефакторинг – серьёзное дело
Поэтому очень важно обеспечить, чтобы после добавления нового кода система продолжила устойчиво работать. В этом плане помогает своевременное обнаружение возможных неполадок в ходе рефакторинга. Ведь никто не хочет столкнуться с ними в продакшене. Так что пишите обширные тесты. Они должны стать важнейшим подспорьем на всём пути от начала и до конца.
Используйте цепочку отслеживания в виде тестов:
- модульных и интеграционных;
- без деталей реализации;
- тестируйте всё по максимуму, чтобы добиться твёрдой уверенности.
В этом случае очень полезны интеграционные тесты, которые способны перехватывать побочные эффекты компонентов.
// Тест компонента React.
it("should show all addons", () => {
// Имитация ответа сервера.
// Утилита, имитирующая минимальный возможный уровень – window.fetch()
server.get(endpoints.loadAddonsData, { addons: ["addon1", "addon2"] });
// Свойство, управляющее кнопкой компонента.
props.shouldShowMoreAddons = true;
// Отрисовка.
render(<Addons {...props} />);
// Клик по кнопке для показа всех аддонов.
fireEvent.click(screen.getByRole("button"));
// Вывод списка аддонов.
await waitFor(() => {
expect(screen.queryByRole("list")).toBeInTheDocument();
});
})Когда все тесты будут пройдены, передавайте код отделу QA для следующего уровня верификации.
▍ Рефакторинг параллельно с добавлением новой функциональности
Если обстоятельства поджимают, запустите новую функциональность, а рефакторинг отложите на потом. Потребуется привлечь отдел QA для повторного тестирования, но это лучше, чем релизить слишком много всего за раз.
Подумайте и лучше не делайте рефакторинг вместе с внесением нового кода, если это предполагает много неизвестных, большие затраты или ведёт к недопустимым рискам. Если же возьмётесь, проследите, чтобы в системе исправно работал ввод-вывод.
Небольшие доработки можно вносить смело.
▍ {} Примеры
В примере ниже показан компонент React
Dashboard, отрисовывающий несколько виджетов и окно дополнительной продажи (<UpsellBox1 />), данные которого определяются вызовом бэкенда (loadUpsell1Data()).// Пример очень упрощён.
export function Dashboard({ data }) {
// Прочая бизнес-логика.
const [shouldShowUpsell1, setShouldShowUpsell1] = useState(false);
const [upsell1Data, setUpsell1Data] = useState(null);
// Прочая бизнес-логика
// …
// …
// …
useEffect(() => {
// Сложная логика для определения, нужно ли показывать UpsellBox1.
if (condition1 && condition2 && !condition3) {
setShouldShowUpsell1(true);
loadUpsell1Data().then((bannerData) => {
setUpsell1Data(bannerData);
});
}
}, [...]);
return (
<div>
<Widget1 />
<Widget2 />
...
{shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
</div>
);
}Представьте, что вам нужно отрисовать второе окно апсейла (
<UpsellBox2 />) и в случае показа обоих оформить их каруселью.return (
<div>
<Widget1 />
<Widget2 />
...
<!-- Показ обоих окон апсейла -->
{shouldShowUpsell1 && shouldShowUpsell2 && (
<UpsellSlider>
<UpsellBox1 data={upsell1Data} />
<UpsellBox2 data={upsell2Data} />
</UpsellSlider>
)}
<!-- ИЛИ только <UpsellBox1 /> -->
{shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
<!-- ИЛИ только <UpsellBox2 /> -->
{shouldShowUpsell2 && <UpsellBox2 data={upsell2Data} />}
</div>
);Поскольку здесь у нас два окна апсейла (в будущем, возможно, больше), логика в
<Dashboard /> разрастается. Есть смысл вынести код апсейла наружу – в кастомный хук useUpsellData (а не в <UpsellSlider />), чтобы сохранить явность – но это будет непросто, поскольку виджеты и окна апсейла смешиваются. Вот как может выглядеть компонент после этих изменений:
// Пример сильно упрощён.
export function Dashboard({ data }) {
// ✅ Тестирование перед рефакторингом.
// ❓ Логика устанавливает «условия», необходимые в useUpsellData.
// Здесь размещается остальная бизнес-логика.
// ✅ Новые тесты, проверяющие, чтобы существующий код не сломался.
// ❓ Подтверждение, что новый код работает.
// ? Высокий риск – оба блока апсейла могут сломаться, что создаст хлопоты для техподдержки и приведёт к потере продаж.
// Тесты хука будут пересекаться с интеграционными тестами Dashboard.
// Это нормально.
const {
upsell1: { shouldShowUpsell1, upsell1Data },
upsell2: { shouldShowUpsell2, upsell2Data },
} = useUpsellData(conditions);
// ✅ Тестирование перед рефакторингом, особенно если последующая
// логика разветвляется апсейлами.
// ❓ Цель – проследить, чтобы существующий код не сломался.
// ? Высокий риск – может нарушить важную функциональность, за чем будет сложно проследить.
// Прочая бизнес-логика
// …
// …
// …
// ✅ Теперь этого useEffect нету, но его реализацию нужно
// протестировать в рамках useUpsellData() ?
// ❓ Цель – проследить, чтобы существующий код не сломался.
// ? Высокий риск – может сломать первый блок апсейла и привести к потере продаж.
// useEffect(() => {
// if (condition1 && condition2 && !condition3) {
// setShouldShowUpsell1(true);
//
// loadUpsell1Data().then((bannerData) => {
// setUpsell1Data(bannerData);
// });
// }
// }, [...]);
return (
<div>
<!--
✅ Тестирование виджетов перед рефакторингом.
❓ Цель – проследить, чтобы существующий код не сломался.
? Риск средний/высокий – может нарушить работу чего-то важного.
-->
<Widget1 />
<Widget2 />
...
<!--
✅ Новые тесты изменений для карусели из двух слайдов.
? Риск высокий/средний – может нарушить существующую функциональность.
-->
{shouldShowUpsell1 && shouldShowUpsell2 && (
<UpsellSlider>
<UpsellBox1 data={upsell1Data} />
<UpsellBox2 data={upsell2Data} />
</UpsellSlider>
)}
<!--
✅ Тестирование конкретного случая перед рефакторингом.
❓ Цель – проследить, чтобы существующий код не сломался.
? Риск высокий/средний – может нарушить существующую функциональность.
-->
{shouldShowUpsell1 && <UpsellBox1 data={upsell1Data} />}
<!--
✅ Новые тесты для проверки изменения.
? Риск средний.
-->
{shouldShowUpsell2 && <UpsellBox2 data={upsell2Data} />}
</div>
);
}Я привёл эти примеры, чтобы показать, насколько неустойчивым может быть рефакторинг при параллельном внесении функциональности в работающий компонент. Проблемы в основном возникают из-за тесно связанной бизнес-логики
<Dashboard />, которая обрабатывает множество виджетов и разделов, выступающих её прямыми потомками.▍ Когда проводить рефакторинг?
- Делайте рефакторинг, если код сильно усложняется, но откажитесь от этого, если не можете подтвердить последующую работоспособность.
- Сопровождайте новую функциональность рефакторингом тех областей, которые в связи с этим будут меняться. Но можно использовать и копи-паст, главное, чтобы это не перерастало в антипаттерн.
- Старайтесь находить новые способы обеспечить прогнозируемость рефакторинга и не полагайтесь на то, что специалисты QA выявят все баги.
- Выносите бизнес-логику из загруженных компонентов, но не бойтесь оставить легаси-код нетронутым, если единственным аргументом для его изменения является то, что он «выглядит неправильно».
Telegram-канал со скидками, розыгрышами призов и новостями IT ?
Комментарии (19)
upcFrost
30.03.2024 18:23+2не полагайтесь на то, что специалисты QA выявят все баги
Более того, QA не резиновые, если их пытаться заставить на каждый чих перепроверять всю систему - очень быстро произойдёт bottleneck в части тестирования, и виноват в нем будет не qa, а разработчик
rukhi7
30.03.2024 18:23+1Более того, QA не резиновые
Вроде как основная задача QA следить за соблюдением процедуры. Если разработчик не знает как проверить то, что он разработал, и не в состоянии хотя бы один раз проверить то, что он разработал по завершению разработки, тогда ничто не поможет проекту с такими разработчиками. QA должны проверять что критерии проверки сформулированы и проверка проведена-реализуема, конечно желательно чтобы QA понимали адекватность проверки.
Если разработчик говорит вам что не знает как проверить то, что он делает/должен сделать, вряд ли можно надеяться, что разработчик понимает предмет под разработкой.
upcFrost
30.03.2024 18:23+1QA должны проверять что критерии проверки сформулированы и проверка проведена-реализуема
Кмк это зависит от того как построен процесс. По моему опыту, гораздо лучше когда QA сам придумывает сценарии и обсуждает их с разработкой (что можно автоматизировать, что нельзя, что опасно и т.д.). По большей части это связано с тем что во время работы над задачей мыслительный процесс разработчика, опять же по моим наблюдениям, сильно отличается от QA: разработчик больше думает со стороны создания, а QA - со стороны возможных косяков. Понятно что разработчик должен и про косяки думать, но в общем случае у QA это получается лучше чисто ввиду разделения труда. Это особенно заметно если бэк и фронт разделены (не full stack), некоторые кейсы могут казаться ппц важными разработчику но для QA это будет откровенно corner case и наоборот.
sshikov
30.03.2024 18:23Вы слегка упрощаете. Я очень часто говорю своим QA, что не знаю, как проверить. И они меня прекрасно понимают - потому что у нас интеграционный проект, и проблемы чаще всего возникают на стыке с другими проектами, причем иногда в результате эксплуатации скажем в течение пары недель.
То есть, чтобы проверить, мне иногда всего-то нужен работающий процесс как в проме, смежный проект с его процессами, люди, которые всем этим пользуются, и все это должно поработать с определенными настройками дней 20. А времени у меня на это не заложено, потому что про влияние времени мы только что в процессе починки бага выяснили. Вот так реально выглядит вариант "не знаю как проверить" - не знаю, как проверить, за разумное время, и не угрохав на это кучу нужных для других задач ресурсов. И при этом само исправление может выглядеть как однострочник.
rukhi7
30.03.2024 18:23Я очень часто говорю своим QA, что не знаю, как проверить. И они меня прекрасно понимают - потому что
Вот интересно и что же вы по результату делаете? Не проверяете? Я примерно это и имел ввиду "не знаю как проверить"="проверять не будем", обычно да -- потому что процес разработки противоречит процесу управления ресурсами, который (процес) контролирует более высокое начальство и который поэтому (а не для пользы дела) имеет более высокий приоритет.
sshikov
30.03.2024 18:23Ну как вам сказать... например высокое начальство мне говорит - вы теперь обязаны проводить еще и нагрузочное тестирование. И еще добавляет - закажите себе стенд для этого тестирования, не экономьте, берите как 100% пром стенда. Мы смотрим на свои пром стенды, выбираем из них самый маленький, и заказываем примерно конфигурацию в 20% от него. И уже от конкретных людей, которые занимаются планированием ресурсов типа процессоров или там дисков получаем замечание: "А вы в курсе, что это будет стоить XX миллионов в год?". Так что процессы на верхних уровнях очень часто слабо стыкуются с реальностью,
Не проверяете?
Если у вас есть код, который точно не работает в определенных условиях (скажем в одном случае из 100). Вы над ним подумали, и поняли, что если изменить одну строчку - то он будет работать правильно и в этом случае, а остальные не заденет (по сути, вы выполнили тест этого куска в голове). И вы можете выдать исправленную версию только тому пользователю, которого затронул этот баг. И процесс, где работает ваш код, не управляет АС, и даже автомобилем. И что вы выберете? Я лично в такой ситуации выберу отдать версию в пром, озвучив риски всем кому надо, и проверить там.
А универсального ответа очевидно тут нет.
rukhi7
30.03.2024 18:23А универсального ответа очевидно тут нет.
Это точно! И даже вопросы у нас маленько расходятся и это тоже нормально, у каждого свой опыт.
19Zb84
Опаснее рефакторинга может быть только преждевременный рефакторинг. А есть по этой теме что нибудь ?
SUNsung
Работает? Не трожь!
atygaev
как же надоели эти громкие высказывания (или утверждения) "Работает? Не трожь!"
Трожь! Трожь как никогда. Без этого не будет прогресса, без этого не будет современного стека и проект утонет в легаси.
Просто все это нужно делать с головой а не просто потому что захотелось
SUNsung
Вот с такой формулировкой точно не трогай.
Если все ради "современного стека" то трогать не нужно.
Рефакторинг необходим если лаги/проблемы, отсутсвие тестов/коментариев или переусложненная реализация. И то это должно быть к месту, а не так что "часовой рефактринг" положил прод на неделю, хотя "у меня все работало"
be_a_dancer
Хороший рефакторинг это тот, который сделан своевременно. Есть как минимум 3 процесса улучшения кода: для исправления техдолга, созданного руками разработчиков, для исправления накопительных ошибок и для исправление запланированного устаревания.
На первый выделяется время в рамках ежедневной работы. Если ты можешь исправить кусок кода в рамках своей задачи - выдели на это время. Если не можешь - выдели задачу. Обычно 10% спринта можно выделить на техдолг.
Второй - очень тяжелый, на него надо выделять время дважды - для обнаружения и для рефакторинга. Нет рецепта.
Исправление запланированного устаревания происходит в рамках обновления библиотек, в рамках обновления фреймворков и экосистемы. Просто обновилось что-то глобальное, выделили время, обновились.
SUNsung
Обычно нужно старатся писать так что бы техдолг не возникал.
Техдолг появляется там где "срочно-срочно" или разрабам вообще похуй кто это потом будет поддерживать.
То что вы написали вторым пунком и есть рефактрринг, а первое это подтирание соплей. Или за разрабами или за менеджером.
Запланированное устаревание очень неоднозначная штука. Бездумно обновлять лишь бы обновлять черевато. А если в проекте много зависимостей обновление зачастую уходит как отлельная большая таска так как нужно сначала свести в единую таблицу что обновляем и что в них изменилось, что бы еше на старте понимать самые острые углы. И делать это должен тот кто хорошо знает проект.
Понятное дело я не имею в виду всякие говнопроекты с 100500 либ где буквально все на либах, даже то что и без либ написалось бы заняв такой же обьем кода. Такие проекты обычно пишутся "один раз" и их поддержка не планировалась на старте. Их и не обновляют обычно потому как "живут" они год-два максимум
be_a_dancer
Вы не правы. Невозможно писать так, чтобы технический долг не возникал, так как:
Мы все люди
В любом живом проекте меняются требования
Мы работаем не одни
Кодовая база крайне редко бывает маленькой. Да, мы все пишем код так, чтобы техдолг не возникал, но меняются подходы, наши знания, да и просто мы растем как специалисты и то, что мы 10 лет назад считали идеальным кодом сейчас не пройдет ревью.
Обновлять зависимости надо всегда, но пусть не в момент выхода, а через пару-тройку патчей.
SUNsung
Все что вы описали как раз второй случай, который нужно сначала накопить, затем исследовать и только потом зная как минимум края можно заниматся рефакторингом.
Если выпускаемый код ПОСТОЯННО требует фиксов и доработок на те самые 10% (не правки, а именно хотфиксы критикалов) то вы явно занимаетесь чем то не тем. Проблема или в разрабах или в самой постановке техпроцесса.
А обновлятся нужно думаючи. Срочно-срочно актуально только если либы получили патчи на zero-уязвимости. В останом случае нужно опять таки подходить к этому думаючи. Пару раз встречал ситуацию когда обновление либы или компилятора затрагивало методы, которые в старой версии более оптимальные по ресурсам/скорости - потому как в новой версии для этих методов сделали "безопасные" аналоги с кучей параметров и плясок, а старые методы так же сделали "безопасными" но ценой всего. И вот тут как раз проще остатся на старой пока не появится окно нормально обновится, переписав критичные места на новые методы, чем бездумно обновится а потом "удивлятся" а чего это все упало.
rukhi7
Это как? Это когда мы рефакторим то чего еще нет? Типа одна команда ведет разработку, а другая сразу разрабатывает переделанную версию, наверно :) .
Или есть еще вариант: у вас же завтра все колом встанет, вот тут посмотрите уже затык!
Ответ: вот когда ВСЕ колом встанет тогда будем рефакторить, нам не нужен преждевременный рефакторинг.
19Zb84
Если рефакторинг в самом начале рпазработки это всегда плохо по определению.
В рефакторинге главное достичь максимально эффективной работы кода, и читаемость уходит на второй план.
Как правило до рефакторинга можно потратить на задачу 1 час. После рефакторинга, эту же задачу можно сделать за 4 часа.
Прогресса в рефакторинге тоже нет. Это процесс улучшенеия технических харрактеристик продукта, а не для удобства разработчика(если только как побочный эффект)
Если качественная архитектура там колом никогда ничего не встает
У реакт разработчиков часто эффект сантехника есть.
Все плохо я сейчас все местами поменяю, и типо все у меня идеал но работает.
Приходит другой и все повторяется..
Я на прошлой неделе работал с реакт проектом с ужасной архитектурой и все супер. Даже не думал о рефакторинге, потому что там рефакторинг это практически смена архитектуры.
Kanut
Ерунда полная. Я всегда сначала пишу "черновик", который просто работает. А потом делаю рефакторинг в контексте производительности, тестируемости, читаемости и так далее и тому подобное.
А вы серьёзно код сразу "начисто" пишите?
rukhi7
иногда бывает что рефакторинг это приведение проекта БЕЗ архитектуры (или с зоопарком обрывков разных архитектур, что, в прочем, то же самое) к какой-то определенной архитектуре.
Скорее всего именно это надо называть рефакторингом, потому что замена одной архитектуры другой тянет на вполне положительную "смену концепции" например.
если качественная архитектура, рефакторинг не нужен, если я правильно понимаю что такое рефакторинг.