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

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

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

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

1. Логика, архитектура и безопасность

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

• Правильно ли работает логика? Есть ли краевые случаи (edge cases)? Что будет, если придет null, undefined или пустой массив?
• Нет ли уязвимостей? В Node.js это особенно актуально. Проверьте, не передаются ли сырые данные из запроса напрямую в SQL запрос (SQL-инъекции) или в eval(). Используются ли санитайзеры для HTML?
• Соблюдена ли архитектура? Не нарушает ли код принципы SOLID? Например, не делает ли одна функция слишком много? Вынесена ли бизнес-логика из обработчиков маршрутов?

// ПЛОХО: Смешивание логики и HTTP слоя
app.get('/user/:id', async (req, res) => {
    const user = await db.query(`SELECT * FROM users WHERE id = ${req.params.id}`); // SQL Injection!
    res.json(user);
});

// ХОРОШО: Разделение ответственности
const getUserById = async (id) => {
    if (!id || typeof id !== 'string') throw new Error('Invalid ID');
    return await db.query('SELECT * FROM users WHERE id = $1', [id]);
};
app.get('/user/:id', async (req, res) => {
    const user = await getUserById(req.params.id);
    res.json(user);
});

2. Асинхронность и работа с промисами

Это ахиллесова пята многих JavaScript-разработчиков. При ревью Node.js кода обязательно проверьте:

• Обработка ошибок. Все ли асинхронные функции обернуты в try/catch? Не забыт ли .catch() у промисов? Висят ли "осиротевшие" промисы (unhandled promise rejections)?
• Правильное использование async/await. Нет ли лишнего await там, где промисы можно запустить параллельно через Promise.all?
• Гонки (race conditions). Может ли код выполниться некорректно при параллельных запросах к одному ресурсу (например, к файлу или глобальной переменной)?

// ПЛОХО: Последовательное выполнение независимых запросов
const user = await getUser(id);
const posts = await getPosts(id);

// ХОРОШО: Параллельное выполнение
const [user, posts] = await Promise.all([getUser(id), getPosts(id)]);

// ПЛОХО: Забытая обработка ошибок
router.get('/data', async (req, res) => {
    const data = await fetchData(); // Если fetchData упадет, сервер "упадет" тоже
    res.json(data);
});

// ХОРОШО
router.get('/data', async (req, res, next) => {
    try {
        const data = await fetchData();
        res.json(data);
    } catch (error) {
        next(error); // Передаем ошибку в глобальный обработчик
    }
});

3. Качество кода, стиль и нейминг

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

• Именование переменных и функций. Названия должны отвечать на вопрос "что это?" и "что делает?". Избегайте однобуквенных переменных (кроме счетчиков циклов) и сокращений вроде getUsrInf.
• Магические числа и строки. Выносятся ли они в константы? const MAX_RETRIES = 3; гораздо понятнее, чем просто 3.
• Размер функций. Функция не должна быть длиннее 20-30 строк. Если она больше — ее нужно декомпозировать.
• Соблюдение стайлгайда. Используется ли ESLint и Prettier? Если да, проверка форматирования — задача линтера, а не человека. Но на ревью стоит обратить внимание на структурные нарушения, которые линтер может не поймать.

// ПЛОХО: Непонятные названия и магические числа
const fn = (a, b) => {
    if (a > 86400) return false;
    return a * b * 0.15;
};

// ХОРОШО: Понятный код
const SECONDS_IN_DAY = 86400;
const DISCOUNT_RATE = 0.15;

const isDiscountValid = (secondsSincePurchase) => {
    return secondsSincePurchase <= SECONDS_IN_DAY;
};
const calculateDiscount = (price, quantity) => {
    return price * quantity * DISCOUNT_RATE;
};

4. Как давать обратную связь: мягкие навыки

Техническая часть — это только половина дела. Вторая половина — это коммуникация. Плохая обратная связь может демотивировать команду. Вот несколько принципов:

• Будьте конкретны. Вместо "этот код плохой" скажите: "В этой функции нет обработки ошибок, что может привести к падению сервера. Давай добавим try/catch".
• Спрашивайте, а не утверждайте. Используйте вопросы: "Почему ты выбрал этот подход?", "Не будет ли здесь проблемы с производительностью?". Это располагает к диалогу.
• Хвалите хорошее. Замечайте не только ошибки, но и удачные решения. "Отличная идея вынести эту логику в отдельный хелпер!"
• Используйте "Я-сообщения". Вместо "Ты написал баг" скажите "Я вижу потенциальную проблему здесь..."
• Не будьте токсичным. Code review — это не соревнование, кто умнее. Это совместная работа над улучшением продукта.