Хороший рефакторинг vs плохой рефакторинг

Я нанял многих разработчиков за последние годы. И не раз случалось, что новички сразу заявляли, что наш код требует серьёзного рефакторинга. Но вот в чём дело: во многих случаях их «улучшенный» код оказывался сложнее для понимания и сопровождения. Более того, он обычно становился медленнее и содержал больше багов.

Не поймите меня неправильно. Рефакторинг — это не что-то плохое само по себе. Это важная часть поддержания кода в хорошем состоянии. Проблема в том, что плохой рефакторинг это действительно плохо. И, к сожалению, попасть в ловушку «хотели как лучше, а получилось как всегда» проще, чем кажется.

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

Хороший, плохой, злой рефакторинг

Абстракции могут быть полезными. А могут быть вредными. Главное — понимать, когда и как их использовать. Давайте рассмотрим распространённые ошибки и способы их избежать.

1. Существенное изменение стиля кода

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

Рассмотрим пример. Представьте, что у нас есть кусок кода, который нужно подкорректировать:

До:

// 🫤 Этот код можно сделать чище function processUsers(users: User[]) {   const result = [];   for (let i = 0; i < users.length; i++) {     if (users[i].age >= 18) {       const formattedUser = {         name: users[i].name.toUpperCase(),         age: users[i].age,         isAdult: true       };       result.push(formattedUser);     }   }   return result; }

Плохой рефакторинг:

import * as R from 'ramda';  // 🚩 Применен совершенно другой стиль и добавлена библиотека const processUsers = R.pipe(   R.filter(R.propSatisfies(R.gte(R.__, 18), 'age')),   R.map(R.applySpec({     name: R.pipe(R.prop('name'), R.toUpper),     age: R.prop('age'),     isAdult: R.always(true)   })) );

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

Хороший рефакторинг:

// ✅ Более чистый и привычный подход function processUsers(users: User[]): FormattedUser[] {   return users     .filter(user => user.age >= 18)     .map(user => ({       name: user.name.toUpperCase(),       age: user.age,       isAdult: true     })); }

Эта версия улучшает исходный код за счёт использования более идиоматичных методов JavaScript, таких как filter и map. Код становится более лаконичным и лёгким для чтения, но при этом не вводит полностью новую парадигму или внешние зависимости.

2. Ненужные абстракции

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

До:

// 🫤 Этот код можно сделать чище function processUsers(users: User[]) {   const result = [];   for (let i = 0; i < users.length; i++) {     if (users[i].age >= 18) {       const formattedUser = {         name: users[i].name.toUpperCase(),         age: users[i].age,         isAdult: true       };       result.push(formattedUser);     }   }   return result; }

Плохой рефакторинг:

// 🚩 Здесь гораздо больше слоев и абстракций, чем необходимо class UserProcessor {   private users: User[];    constructor(users: User[]) {     this.users = users;   }    public process(): FormattedUser[] {     return this.filterAdults().formatUsers();   }    private filterAdults(): UserProcessor {     this.users = this.users.filter(user => user.age >= 18);     return this;   }    private formatUsers(): FormattedUser[] {     return this.users.map(user => ({       name: this.formatName(user.name),       age: user.age,       isAdult: true     }));   }    private formatName(name: string): string {     return name.toUpperCase();   } }  const processUsers = (users: User[]): FormattedUser[] => {   return new UserProcessor(users).process(); };

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

Хороший рефакторинг:

// ✅ Более чистый и привычный подход const isAdult = (user: User): boolean => user.age >= 18;  const formatUser = (user: User): FormattedUser => ({   name: user.name.toUpperCase(),   age: user.age,   isAdult: true });  function processUsers(users: User[]): FormattedUser[] {   return users.filter(isAdult).map(formatUser); }

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

3. Добавление несогласованности

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

Предположим, у нас есть React-приложение, где для работы с данными мы последовательно используем React Query:

// По всему приложению import { useQuery } from 'react-query';  function UserProfile({ userId }) {   const { data: user, isLoading } = useQuery(['user', userId], fetchUser);    if (isLoading) return <div>Loading...</div>;   return <div>{user.name}</div>; }

Теперь представьте, что разработчик решает использовать Redux Toolkit только для одного компонента:

// Единоразовый компонент import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { fetchPosts } from './postsSlice';  function PostList() {   const dispatch = useDispatch();   const { posts, status } = useSelector((state) => state.posts);    useEffect(() => {     dispatch(fetchPosts());   }, [dispatch]);    if (status === 'loading') return <div>Loading...</div>;   return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>; }

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

Лучший подход — продолжать использовать React Query:

// Единообразный подход import { useQuery } from 'react-query';  function PostList() {   const { data: posts, isLoading } = useQuery('posts', fetchPosts);    if (isLoading) return <div>Loading...</div>;   return <div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>; }

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

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

4. Не разобраться в коде перед рефакторингом

Одна из самых больших проблем, которые я наблюдал, — рефакторинг кода в процессе его изучения, с целью «лучше понять его». Это ужасная идея. Я встречал рекомендации работать с конкретным фрагментом кода в течение 6-9 месяцев перед его изменением. В противном случае ваши изменения, скорее всего, приведут к появлению багов, ухудшению производительности и так далее.

До:

// 🫤 Здесь слишком много жестко закодированного function fetchUserData(userId: string) {   const cachedData = localStorage.getItem(`user_${userId}`);   if (cachedData) {     return JSON.parse(cachedData);   }    return api.fetchUser(userId).then(userData => {     localStorage.setItem(`user_${userId}`, JSON.stringify(userData));     return userData;   }); } 

Плохой рефакторинг:

// 🚩 Куда делось кеширование? function fetchUserData(userId: string) {   return api.fetchUser(userId); }

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

Хороший рефакторинг:

// ✅ Более чистый код с сохранением текущего поведения async function fetchUserData(userId: string) {   const cachedData = await cacheManager.get(`user_${userId}`);   if (cachedData) {     return cachedData;   }    const userData = await api.fetchUser(userId);   await cacheManager.set(`user_${userId}`, userData, { expiresIn: '1h' });   return userData; }

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

5. Понимание бизнес-контекста

Однажды я присоединился к компании, которая испытывала серьёзные затруднения из-за большого объёма легаси-кода. Я возглавил проект по миграции интернет-магазина на новую, более современную, быструю и, как мне казалось, лучшую технологию… Angular.js.

Оказалось, что бизнес сильно зависел от SEO, а мы создали медленное и раздутое одностраничное приложение (SPA).

В результате за два года мы не выпустили ничего, кроме более медленной, забагованной и менее поддерживаемой копии сайта. Почему? Люди, которые возглавляли этот проект (я — главный виновник в этой ситуации), не имели опыта работы с этим сайтом. Я был молод и неопытен.

Давайте рассмотрим современный пример этой ошибки:

Плохой рефакторинг:

// 🚩 Одностраничное приложение для сайта, ориентированного на SEO, — плохая идея function App() {   return (     <Router>       <Switch>         <Route path="/product/:id" component={ProductDetails} />       </Switch>     </Router>   ); }

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

Хороший рефакторинг:

// ✅ Серверный рендеринг для сайта, ориентированного на SEO export const getStaticProps: GetStaticProps = async () => {   const products = await getProducts();   return { props: { products } }; };  export default function ProductList({ products }) {   return (     <div>       ...     </div>   ); }

Подход на основе Next.js предоставляет серверный рендеринг «из коробки», что критически важно для SEO. Он также обеспечивает лучший пользовательский опыт за счёт более быстрой первоначальной загрузки страницы и улучшенной производительности для пользователей с медленным соединением. Remix тоже подходит для этой задачи, предлагая аналогичные преимущества для серверного рендеринга и оптимизации SEO.

6. Чрезмерная консолидация кода

Однажды я нанял человека, который уже в первый день работы над нашим бэкендом сразу взялся за рефакторинг кода. У нас было несколько функций Firebase, у каждой из которых были свои настройки, такие как тайм-ауты и объём памяти.

Вот как выглядела наша исходная настройка.

До:

// 😕 Этот код у нас повторялся более 40 раз в кодовой базе, возможно, стоит его объединить export const quickFunction = functions   .runWith({ timeoutSeconds: 60, memory: '256MB' })   .https.onRequest(...);  export const longRunningFunction = functions   .runWith({ timeoutSeconds: 540, memory: '1GB' })   .https.onRequest(...);

Разработчик решил обернуть все функции в одну функцию createApi.

Плохой рефакторинг:

// 🚩 Бездумное объединение настроек, которые не должны быть объединены const createApi = (handler: RequestHandler) => {   return functions     .runWith({ timeoutSeconds: 300, memory: '512MB' })     .https.onRequest((req, res) => handler(req, res)); };  export const quickFunction = createApi(handleQuickRequest); export const longRunningFunction = createApi(handleLongRunningRequest);

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

Лучшим подходом было бы разрешить передавать опции Firebase через API.

Хороший рефакторинг:

// ✅ Установить хорошие значения по умолчанию, но позволить их переопределять const createApi = (handler: RequestHandler, options: ApiOptions = {}) => {   return functions     .runWith({ timeoutSeconds: 300, memory: '512MB', ...options })     .https.onRequest((req, res) => handler(req, res)); };  export const quickFunction = createApi(handleQuickRequest, { timeoutSeconds: 60, memory: '256MB' }); export const longRunningFunction = createApi(handleLongRunningRequest, { timeoutSeconds: 540, memory: '1GB' });

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

И, серьёзно, разберитесь в коде, прежде чем пытаться его «улучшить». У нас возникли проблемы при следующем развёртывании некоторых API, которых можно было избежать, если бы не этот необдуманный рефакторинг.

Как правильно проводить рефакторинг

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

Вот несколько советов для успешного рефакторинга:

1. Работайте постепенно: вносите небольшие, управляемые изменения вместо полного переписывания.

2. Тщательно изучите код: перед внесением значительных измнений или добавлением новых абстракций убедитесь, что вы глубоко понимаете, как работает текущий код.

3. Соблюдайте существующий стиль кода: согласованность — ключ к поддерживаемости.

4. Избегайте избыточных абстракций: сохраняйте простоту, если сложность не оправдана.

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

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

7. И, наконец, прививайте эти принципы своим коллегам. 

Инструменты и техники для улучшения рефакторинга

Инструменты линтинга

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

Код-ревью

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

Тестирование

Проводите тесты, чтобы убедиться, что рефакторинг не ломает существующую функциональность.

• Vitest: быстрый, надёжный и простой в использовании тестовый раннер, который работает «из коробки» без дополнительной настройки.

• Storyboo: подходит для визуального тестирования.

• React Testing Library: набор утилит для тестирования компонентов React (есть аналогичные версии для Angular и других фреймворков).

ИИ-инструменты 

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

Инструмент, особенно полезный для поддержания консистентности при разработке фронтенда, — Visual Copilot. Это инструмент на базе искусственного интеллекта, который помогает преобразовывать дизайны в код, сохраняя ваш стиль и корректно используя элеметы и переменные (токены) дизайн-системы.

Заключение

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

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

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

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

Больше полезных навыков по разработке и тестированию вы можете получить в рамках практических онлайн-курсов от экспертов отрасли. Также в рамках курсов проходят бесплатные открытые уроки по всем IT-направлениям — переходите в календарь и выбирайте актульные для себя темы.