Code Review в JavaScript: что проверять и как давать обратную связь на практике

Введение: почему code review — это не про "найти баги"

Code review (ревью кода) — это не просто формальность или способ найти опечатки. В современной разработке на JavaScript и Node.js это главный инструмент для поддержания качества кода, обмена знаниями в команде и предотвращения технического долга. Однако многие разработчики сводят ревью к проверке синтаксиса, упуская из виду архитектурные проблемы, производительность и читаемость.

В этой статье мы разберем, что именно проверять в JavaScript-коде (на примере Node.js), и как давать обратную связь так, чтобы не обидеть коллегу, а помочь ему вырасти. Практические примеры кода помогут закрепить материал.

1. Что проверять в JavaScript-коде: практический чек-лист

Начнем с конкретных точек проверки. Разделим их на три уровня: синтаксис/стиль, логика/безопасность и архитектура.

1.1. Стиль кода и форматирование

Здесь все просто: используйте линтер (ESLint) и форматер (Prettier). На ревью не должно быть споров о пробелах. Настройте конфиги в проекте один раз. Проверяйте, что:

• Код соответствует правилам eslint-config-airbnb или вашим кастомным настройкам.
• Нет лишних console.log (если это не dev-режим).
• Имена переменных осмысленны: const userData, а не const x.

1.2. Логика работы с асинхронностью (Promise, async/await)

Это частая зона ошибок в Node.js. Проверяйте:

• Обработку ошибок в асинхронных функциях. Отсутствие try/catch может привести к падению процесса.
• Правильное использование await внутри циклов. Если задачи не зависят друг от друга, используйте Promise.all.

Плохо:

// Последовательное выполнение, хотя можно параллельно
const results = [];
for (const id of ids) {
  const data = await fetchData(id); // Ждем каждый запрос
  results.push(data);
}

Хорошо:

// Параллельное выполнение
const promises = ids.map(id => fetchData(id));
const results = await Promise.all(promises);

1.3. Безопасность и утечки памяти

Для Node.js критично:

• Отсутствие инъекций: не используйте eval или new Function без острой необходимости.
• Закрытие потоков (streams), соединений с БД и файловых дескрипторов.
• Валидация входных данных. Никогда не доверяйте телу запроса (req.body).

2. Как давать обратную связь: психология и тактика

Даже идеальный список проверок не сработает, если обратная связь (feedback) вызывает у автора кода желание закрыть PR и уволиться. Вот несколько правил.

2.1. Правило "сэндвича" (необязательное, но работающее)

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

• "Отличное решение с использованием Map для кэширования!"
• "Однако здесь есть потенциальная проблема с мутацией исходного массива. Давай используем .map() вместо .forEach() с push."
• "В остальном код чистый, спасибо за быстрый фикс!"

2.2. Комментируйте код, а не человека

Вместо "Ты написал ужасный цикл" скажите "Этот цикл можно заменить на Array.reduce для улучшения читаемости". Избегайте местоимений "ты" в негативном контексте.

2.3. Давайте контекст и альтернативы

Просто написать "Это неправильно" — плохо. Объясните почему и предложите вариант. Пример плохого комментария:

"Не используй var."

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

"Здесь var создает переменную в глобальной области видимости, что может привести к багам. Давай заменим на const, так как значение не переприсваивается."

3. Практический пример: ревью реального кода на Node.js

Допустим, коллега прислал такой код для обработки запроса:

// userController.js
const getUsers = async (req, res) => {
  try {
    const users = await User.find();
    res.json(users);
  } catch (err) {
    console.log(err);
    res.status(500).send('Error');
  }
};

Что проверяем?

1. Отсутствие фильтрации: Запрос возвращает всех пользователей. Нужна пагинация или хотя бы лимит.
2. Логгирование: console.log в продакшене — плохая практика. Используйте логгер (winston, pino).
3. Обработка ошибок: Сообщение 'Error' неинформативно. Лучше возвращать конкретный код и сообщение.
4. Отсутствие валидации: Нет проверки, авторизован ли пользователь.

Как дать обратную связь?

• Начните с позитива: "Хорошо, что используется async/await и try/catch."
• Укажите на проблемы по пунктам, предлагая решения: "Для безопасности предлагаю добавить мидлвару аутентификации. Также заменим console.log на логгер."
• Предложите рефакторинг: "Можно вынести логику в сервисный слой для тестируемости."

4. Автоматизация ревью: что можно оставить машине

Не тратьте время ревьювера на то, что может проверить линтер или CI. Настройте:

• ESLint — для стиля и потенциальных ошибок.
• Prettier — для форматирования.