Что бы не хотел видеть на код ревью?

Что я категорически не хотел бы видеть на код ревью

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

1. Код без явной цели и контекста

Самое раздражающее — это пулл-реквест (PR) или коммит без описания. Когда автор просто присылает код с заголовком "фикс" или "новый функционал", ревьюер тратит огромное количество времени на понимание:

• Что именно было изменено?
• Какую бизнес-проблему это решает?
• Какие были требования?

Пример плохого описания в PR:

## Фикс бага

Пример хорошего описания:

## Исправление утечки памяти в компоненте модального окна
**Проблема:** При закрытии модального окна через `setTimeout` не очищались подписки на события `keydown`.
**Решение:** Добавлен метод `cleanupListeners` в lifecycle `componentWillUnmount`.
**Ссылка на задачу:** JIRA-1234.

2. Массивные, неразделенные изменения

Один коммит или PR, который содержит:

• Рефакторинг
• Новый функционал
• Исправление багов
• Обновление конфигураций

Это делает ревью практически невозможным. Риски пропустить критическую ошибку возрастают экспоненциально. Изменения должны быть инкапсулированными и атомарными.

3. Игнорирование общепринятых стандартов проекта

Каждый проект имеет свои:

• Conventions (например,命名约定 для переменных)
• Линтеры и правила форматирования (ESLint, Prettier)
• Архитектурные паттерны (например, использование конкретного state management)

Код, который нарушает эти правила, вызывает вопросы: "Почему здесь snake_case, когда весь проект использует camelCase?"

// Плохо: нарушение соглашений проекта
const user_name = 'John'; // проект использует camelCase

// Хорошо: соответствие соглашениям
const userName = 'John';

4. "Магические числа" и непонятные конструкции

Код, который требует дешифровки:

• Числа или строки без объяснения их значения
• Сложные логические условия без комментариев
• Неочевидные алгоритмы

// Плохо: что такое 86400000?
const timeout = 86400000;

// Хорошо: понятная константа
const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000;
const timeout = ONE_DAY_IN_MS;

5. Отсутствие обработки ошибок и edge cases

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

• Нестабильными сетевыми запросами
• Пользовательским вводом
• Различными браузерными окружениями

Плохой пример (React компонент):

async function fetchData() {
  const response = await fetch('/api/data');
  const data = await response.json(); // нет проверки на успешность response!
  setState(data);
}

Хороший пример:

async function fetchData() {
  try {
    const response = await fetch('/api/data');
    if (!response.ok) {
      throw new Error(`HTTP error! status: ${response.status}`);
    }
    const data = await response.json();
    setState(data);
  } catch (error) {
    setErrorState(error.message);
    logError(error);
  }
}

6. "Копипаст" кода без понимания

Часто встречаются куски кода, взятые из других файлов или даже из интернета, которые:

• Не адаптированы к контексту проекта
• Содержат избыточные или неиспользуемые части
• Могут иметь скрытые баги или уязвимости

7. Комментарии, которые не поясняют, а конфузят

Комментарии типа:

• "Здесь это работает, не трогать" (почему?)
• "Фикс для странного бага" (какого?)
• Закомментированный старый код, который никто не удаляет

8. Игнорирование performance implications

На фронтенде performance — это ключевой пользовательский опыт. Код, который:

• Создает неоптимальные повторные рендеры в React
• Использует тяжелые операции в цикле
• Не использует debouncing/throttling для частых событий
• Загружает огромные библиотеки без необходимости

// Плохо: потенциально тяжелая операция на каждый рендер
function Component({ list }) {
  const sortedList = list.sort((a, b) => a.value - b.value); // sort мутирует исходный массив и вызывается каждый рендер!

  return <div>{sortedList.map(item => <span>{item.name}</span>)}</div>;
}

// Хорошо: использование memoization или оптимизированных вычислений
function Component({ list }) {
  const sortedList = useMemo(() => [...list].sort((a, b) => a.value - b.value), [list]);

  return <div>{sortedList.map(item => <span>{item.name}</span>)}</div>;
}

9. Несоответствие принципам доступности (a11y)

Фронтенд-код должен быть доступным. Отсутствие:

• Соответствующих ARIA-атрибутов
• Семантической HTML-структуры
• Управления с клавиатуры
• Поддержки screen readers

Это не просто нарушение best practices, но и потенциальные юридические риски для проекта.

10. Пропущенная безопасность (security)

Особенно в современных фронтенд-приложениях с интерактивными формами и динамическим контентом. Опасные паттерны:

• Инъекции через innerHTML без санации
• Exposure чувствительных данных в клиентском коде
• Невалидированный пользовательский ввод, отправляемый на сервер

Мое итоговое мнение

Код ревью — это коллективная ответственность. Автор кода должен сделать свою работу максимально понятной и подготовленной для ревью. Ревьюер должен внимательно анализировать не только функциональность, но и долгосрочные последствия кода: поддерживаемость, performance, безопасность и согласованность.

Когда эти принципы нарушаются, процесс ревью становится обременительным и малоэффективным. Идеальный код для ревью — это тот, который сам рассказывает свою историю: почему он был написан, как он работает, и какие решения были приняты. Это требует дисциплины и усилий, но именно это отличает профессиональную разработку от хаотичного кодирования.