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

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

Code review — это не просто формальность, а мощный инструмент повышения качества кода. В C++ ревью особенно важно из-за сложности языка: ручное управление памятью, шаблоны, множественное наследование. В этой статье мы разберем, на что обращать внимание при проверке C++ кода и как формулировать замечания, чтобы они были полезными, а не обидными.

1. Безопасность памяти и RAII

Первое, что нужно проверить — управление ресурсами. В C++ нет сборщика мусора, поэтому утечки памяти и двойное освобождение — частая проблема. Ключевой паттерн — RAII (Resource Acquisition Is Initialization).

Что проверять:

• Используются ли умные указатели (std::unique_ptr, std::shared_ptr) вместо голых new/delete?
• Нет ли ручного вызова delete в коде? (Кроме редких случаев низкоуровневой работы)
• Правильно ли реализованы RAII-классы: ресурс захватывается в конструкторе и освобождается в деструкторе?
• Не забыт ли виртуальный деструктор в базовом классе?

// ❌ Плохо: ручное управление
void process() {
    int* data = new int[100];
    // ... если будет исключение, память утечет
    delete[] data;
}

// ✅ Хорошо: RAII через std::vector
void process() {
    std::vector<int> data(100);
    // безопасно при исключениях
}

Как давать обратную связь: Вместо «Ты забыл удалить память» скажите: «Здесь лучше использовать std::vector или std::unique_ptr — это автоматически освободит ресурсы и сделает код исключениебезопасным».

2. Const-correctness и ссылки

C++ позволяет явно указывать, что данные не должны меняться. Это повышает читаемость и позволяет компилятору лучше оптимизировать код.

Что проверять:

• Все ли параметры, которые не изменяются, помечены const?
• Передаются ли большие объекты по константной ссылке (const T&), а не по значению?
• Помечены ли методы, не изменяющие объект, как const?
• Не используется ли const_cast без крайней необходимости?

// ❌ Плохо: копирование большого объекта
std::string toUpper(std::string s); // копия

// ✅ Хорошо: константная ссылка
std::string toUpper(const std::string& s); // без копирования

// ❌ Плохо: метод не const, хотя не меняет состояние
class Point {
    int x, y;
public:
    int getX() { return x; } // должно быть const
};

// ✅ Хорошо
int getX() const { return x; }

Как давать обратную связь: «Давай добавим const для параметров, которые не изменяются — это сделает интерфейс понятнее и предотвратит случайные изменения».

3. Использование STL и алгоритмов

Стандартная библиотека C++ содержит множество готовых решений. Изобретение велосипедов — частая ошибка новичков.

Что проверять:

• Нет ли ручных циклов, которые можно заменить алгоритмами (std::find, std::sort, std::transform)?
• Правильно ли выбран контейнер? (std::vector для произвольного доступа, std::list для частых вставок в середину)
• Используются ли std::string_view для передачи строк без копирования?
• Не забыты ли reserve() для std::vector при известном размере?

// ❌ Плохо: ручной поиск
bool contains(const std::vector<int>& v, int val) {
    for (size_t i = 0; i < v.size(); ++i) {
        if (v[i] == val) return true;
    }
    return false;
}

// ✅ Хорошо: использование STL
bool contains(const std::vector<int>& v, int val) {
    return std::find(v.begin(), v.end(), val) != v.end();
}

Как давать обратную связь: «Этот цикл можно заменить на std::find_if — код станет короче и выразительнее. Алгоритмы STL обычно оптимизированы лучше ручных циклов».

4. Производительность и микрооптимизации

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

Что проверять:

• Нет ли копирования вместо перемещения (используйте std::move)?
• Не передаются ли строки по значению без необходимости?
• Не вызываются ли виртуальные функции в горячих циклах?
• Не заблокирован ли мьютекс дольше, чем нужно?

// ❌ Плохо: лишнее копирование
void process(std::string s); // копия

// ✅ Хорошо: передача по значению + move
void process(std::string s) {
    std::string local = std::move(s); // перемещение
}

// ❌ Плохо: виртуальный вызов в цикле
for (auto& obj : objects) {
    obj->doWork(); // виртуальный вызов
}

// ✅ Хорошо: если возможно, используйте CRTP или std::variant

Как давать обратную связь: «Здесь происходит лишнее копирование строки. Давай передадим её по значению и используем std::move — это ускорит код на 20-30% без потери читаемости».

5. Формулировка обратной связи

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

Правила хорошего code review:

• Будьте конкретны. Вместо «Этот код плохой» — «З