Ревью кода на C++: за что senior-ы цепляются в первую очередь

Онлайн-тренажер Python для начинающих

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

Начать курс

Введение: почему ваше ревью превращается в поле боя?

Вы отправили Merge Request. Вроде всё чисто, логика верная, код компилируется. Через час приходит ответ от сеньора: «-10 баллов Гриффиндору» и 35 комментариев к 200 строкам кода. Знакомо?

Ревью кода на C++ — это не просто проверка синтаксиса. Это искусство, которое разделяет junior-ов и senior-ов. C++ даёт разработчику огромную власть над памятью и производительностью, но с великой властью приходит великая ответственность за неопределённое поведение и утечки ресурсов.

В этой статье мы заглянем в голову опытного ревьюера. Я собрал реальные замечания, которые senior-ы оставляют на code review в продакшн-проектах. Если вы пишете на C++ и хотите, чтобы ваши PR принимали с первого раза — читайте внимательно. А если вы тимлид — используйте этот чек-лист для онбординга новичков.

1. Выстрел в ногу: ручное управление памятью

Самый частый холивар на ревью — "почему ты не используешь умные указатели?". Senior-ы ненавидят голые new и delete в современном C++ (C++11 и новее).

Плохой пример

class DataProcessor {
    int* buffer;
    size_t size;
public:
    DataProcessor(size_t s) : size(s) {
        buffer = new int[s]; // Кто будет удалять?
    }

    ~DataProcessor() {
        delete[] buffer; // А если исключение в конструкторе?
    }
};

Комментарий senior-а: "Нарушение RAII. Если конструктор выбросит исключение после выделения памяти — деструктор не вызовется. Утечка памяти. Используй std::vector или std::unique_ptr."

Правильный пример

class DataProcessor {
    std::vector<int> buffer;
public:
    explicit DataProcessor(size_t s) : buffer(s) {}
};

Сеньоры обожают RAII (Resource Acquisition Is Initialization). Если вы пишете new в коде, будьте готовы объяснить, почему std::make_unique или std::vector вам не подходят. В 95% случаев — подходят.

2. Const-correctness: поставьте себе «запятую»

Пропущенный const — это красная тряпка для любого senior-разработчика на C++. Это не просто стиль, это контракт с читателем кода и компилятором.

Что бесит на ревью

std::string getName() { // Почему не const?
    return name;
}

void printData(std::vector<int>& data) { // Копия? Ссылка? Изменяет?
    for (auto& item : data) {
        std::cout << item;
    }
}

Комментарий senior-а: "Метод getName() не изменяет объект — сделай его const. Параметр data не изменяется — передавай const std::vector<int>&. Иначе вызывающий код не знает, изменишь ты его данные или нет."

Исправляем

std::string getName() const { // const метод
    return name;
}

void printData(const std::vector<int>& data) { // const ссылка
    for (const auto& item : data) {
        std::cout << item;
    }
}

Правило простое: если метод не меняет состояние объекта — const. Если функция не меняет аргумент — const T&. Const correctness в C++ — это не опция, это стандарт.

3. Move-семантика: копировать — не наша религия

Современный C++ — это про перемещение, а не копирование. Senior-ы сразу видят, где вы создаёте лишние копии и убиваете производительность.

Плохой паттерн

std::vector<int> processData(std::vector<int> data) { // Копия на входе
    // ... много логики ...
    return data; // Копия на выходе? Или NRVO?
}

void caller() {
    std::vector<int> bigData = {1, 2, 3, ... 1000000};
    auto result = processData(bigData); // Ещё одна копия!
}

Комментарий senior-а: "Две лишних копии вектора по 4 МБ. Передавай по значению только если ты всё равно собираешься сделать копию внутри. Иначе — const T&. А для возврата используй std::move или полагайся на RVO."

Оптимальный вариант

std::vector<int> processData(const std::vector<int>& data) {
    std::vector<int> result;
    result.reserve(data.size());
    // ... заполняем result ...
    return result; // RVO сработает, копии не будет
}

Ещё один частый косяк: передача std::unique_ptr по константной ссылке. Если функция забирает владение — принимайте по значению и используйте std::move. Если только смотрит — const T& или сырой указатель (но с умом!).

4. Неопределённое поведение: тихий убийца

Senior-разработчики имеют радар на undefined behavior. Код может работать на вашей машине, но упасть на сервере. Вот классика.

Пример: переполнение signed int

int calculateSum(int a, int b) {
    return a + b; // Если a и b — большие числа, переполнение — UB!
}

Комментарий senior-а: "Стандарт C++ говорит: signed integer overflow — это undefined behavior. Компилятор может оптимизировать так, что твой код сделает что угодно. Используй std::optional или проверяй границы."

Пример: висячие ссылки

std::string& getLocalString() {
    std::string local = "hello";
    return local; // UB! local уничтожится при выходе из функции
}

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