На что обращаешь внимание при Code Review?

Ключевые принципы и фокусы при проведении Code Review

Как Senior Frontend Developer, я рассматриваю Code Review не как формальную проверку, а как коллективный процесс улучшения кода, направленный на повышение качества, поддерживаемости и знаний команды. Моё внимание распределяется между несколькими критическими уровнями.

1. Функциональная корректность и логика

Первичная задача — убедиться, что код выполняет свою функцию правильно и без ошибок.

• Краевые случаи (Edge Cases): Проверяю обработку нестандартных входных данных (null, undefined, пустые строки/массивы, крайние значения).
• Состояние ошибок (Error State): Анализирую, как код реагирует на ошибки (сети, валидации) и предоставляет пользователю понятные сообщения.
• Логические ошибки: Ищу некорректные условия в if/else, циклы с неправильными пределами, неправильное использование операторов (&&, ||).

// Пример: Проверка краевого случая
// Плохо: может вызвать ошибку при arr === null
function getFirstElement(arr) {
    return arr[0];
}

// Хорошо: явная проверка
function getFirstElement(arr) {
    if (!Array.isArray(arr) || arr.length === 0) {
        return null; // или throw new Error()
    }
    return arr[0];
}

2. Читаемость, понятность и соблюдение соглашений

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

• Соглашения по стилю (Styling Conventions): Следует ли код принятым в проекте правилам (отступы, именование, длина строк)?
• Ясность именования (Clear Naming): Имена переменных, функций и компонентов должны точно отражать их суть (userData vs data, handleSubmit vs onClick).
• Избыточная сложность: Разбиты ли сложные функции и компоненты? Не используется ли "магическое число" (Magic Number) или сложные трюки, которые можно выразить яснее?

3. Архитектура и проектные решения

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

• Выбор правильных абстракций: Используется ли уместный уровень абстракции? Не происходит ли "утечка абстракций (Leaky Abstraction)", когда детали реализации выходят наружу?
• Разделение ответственности (Separation of Concerns): Проверяю, соблюдаются ли принципы SOLID (особенно Single Responsibility). Компонент должен отвечать за одну вещь.
• Состояние (State Management): Локальное состояние хранится там, где нужно? Не происходит ли "проп drilling (Prop Drilling)", когда данные передаются через множество промежуточных компонентов? Возможно, стоит использовать Context, React Query или состояние на уровне страницы.
• Повторное использование (Reusability): Можно ли выделить повторяющуюся логику или UI в отдельный хук, компонент или функцию?
• Интеграция с существующим кодом: Новый код гармонично встраивается в текущую архитектуру или создаёт конфликтующие подходы?

4. Performance и оптимизация (для Frontend)

В современном фронтенде производительность — ключевой критерий пользовательского опыта.

• Неоптимальные рендеры (Re-renders): В React анализирую, не вызываются ли лишние ререндеры из-за неправильного использования состояния, отсутствия мемоизации (useMemo, useCallback) или тяжелых вычислений внутри компонента.
• Большие или частые сетевые запросы: Не загружаются ли данные, которые уже есть? Не делаются запросы на каждый рендер? Оптимально ли используется кэширование (например, через React Query или SWR)?
• Размер бандла (Bundle Size): Не импортируется ли огромная библиотека для одной маленькой функции? Возможно, стоит использовать tree-shakable альтернативы или динамический импорт (import()).

// Пример: Избегание лишних ререндеров в React
// Плохо: новая функция создается при каждом рендере, вызывая ререндер Child
function ParentComponent() {
    const handleClick = () => console.log('Clicked');
    return <ChildComponent onClick={handleClick} />;
}

// Хорошо: функция мемоизируется
function ParentComponent() {
    const handleClick = useCallback(() => console.log('Clicked'), []);
    return <ChildComponent onClick={handleClick} />;
}

5. Безопасность и защита данных

Часто недооцениваемый, но критически важный аспект.

• Инъекции (Injections): Проверяю, что динамический контент (например, из пользовательского ввода) безопасно рендерится, предотвращая XSS (Cross-Site Scripting).
• Уязвимости зависимостей (Dependency Vulnerabilities): Новые библиотеки не содержат известных уязвимостьей? (Это часто проверяется автоматически, но стоит обратить внимание).
• Незащищённые данные: Критические данные (токены, ключи) не попадают в клиентский код или публичные репозитории.

6. Тестируемость и сопровождение

Код должен быть готов к тестам и будущим изменениям.

• Наличие тестов: Код покрыт тестами? Если не покрыт — можно ли его легко покрыть? Сложная, запутанная логика часто не тестируется.
• Модифицируемость (Modifiability): Можно ли легко изменить или расширить этот код в будущем? Не слишком он "зацеплен (Coupled)" с другими частями системы?
• Комментарии и документация: Комментарии объясняют "почему", а не "что". Сложные алгоритмы или бизнес-правила имеют пояснения.

Мой подход к процессу

Я стараюсь давать конструктивные и конкретные комментарии, предлагая альтернативы или спрашивая о причинах выбора ("Почему был выбран этот подход?"). Я избегаю субъективных утверждений ("Это плохо") и вместо этого говорю о потенциальных проблемах ("Это может вызвать лишние ререндеры при..."). Главная цель Code Review — совместное создание лучшего кода и рост команды, а не поиск ошибок.