Ответ
Да, я регулярно участвую в процессе Code Review как в роли автора, так и в роли рецензента. В моих проектах на C++ это обязательный этап перед мержем в основную ветку.
При ревью кода на C++ я фокусируюсь на нескольких ключевых аспектах:
- Безопасность памяти и ресурсов: Проверяю, что нет утечек памяти (используются
std::unique_ptr/std::shared_ptrвместо сырых указателей где возможно), корректно обрабатываются исключения (RAII), нет выхода за границы контейнеров (используются.at()или проверки). - Стиль и читаемость: Соблюдение принятого в команде стиля (например, основанного на Google C++ Style Guide или Core Guidelines). Имена переменных, методов должны быть понятными.
- Производительность: Отсутствие ненужных копирований объектов (проверка на возможность использования move-семантики, передача по const-ссылке), оптимальный выбор структур данных (например,
std::vectorvsstd::list). - Современность кода (Modern C++): Поощряю использование возможностей C++11/14/17/20, где это уместно (auto, range-based for, structured bindings,
std::optional).
Пример замечания:
// Было: риск утечки памяти при выбросе исключения
void processOldStyle(Resource* res) {
res->acquire();
// ... код, который может выбросить исключение
res->release();
}
// Стало: использование RAII
void processModern() {
auto res = std::make_unique<Resource>(); // Умный указатель
res->acquire();
// ... код
// release() вызовется в деструкторе Resource автоматически
}Я всегда стараюсь давать конструктивную обратную связь, объясняя не только что не так, но и почему и как это можно улучшить, ссылаясь на стандарт или best practices.
Ответ 18+ 🔞
А, ну ты про ревью кода спрашиваешь? Да, конечно, участвую, и ещё как. И сам код пихаю на проверку, и чужие творения разбираю. В C++ это вообще святое, иначе потом такой пиздец разгребать, что мама не горюй — овердохуища работы.
Когда я код смотрю, у меня в голове сразу чек-лист включается, как у параноика. Первое и главное — безопасность памяти и ресурсов. Смотрю в оба, чтобы не было классических ловушек. Если вижу сырой new без обёртки — сразу подозрение ёбать чувствую. Где твой std::unique_ptr, чувак? А если исключение вылетит посередине — кто память освобождать будет? Призрак из прошлого? Используй RAII, ёпта, это же основа основ. И чтобы за границы вектора не вылезали — либо .at(), который тебя швырнёт, либо свои проверки. Волнение ёбать, когда видишь array[i] в цикле без всяких границ.
Дальше — стиль и читаемость. Это, конечно, дело вкуса, но если в команде договорились на один стиль — его и блюди. Меня бесят имена переменных вроде a, tmp2 или qwerty. Ты чё, клавиатуру тестишь? Называй вещи своими именами, чтобы через полгода самому не охуеть от своего же кода. И отступы, блядь, расставь нормально, а то читать невозможно — глаза сломаешь.
Производительность — отдельная песня. C++ же не для того придуман, чтобы тормозить. Вижу, что огромный объект по значению туда-сюда гоняют — сразу вопрос: «Мужик, а нахуя?». Давай const-ссылку, давай move-семантику, если можно. И структуры данных выбирай с умом. std::list для всего подряд — это пиздопроебибна идея, в 90% случаев std::vector будет быстрее. Надо думать головой, э бошка думай!
Ну и современность. Сижу, смотрю на код, будто на дворе 2002-й год. NULL вместо nullptr, свои велосипеды вместо std::optional, ручные циклы там, где можно range-based for. Ребята, мы уже в будущем живём, стандарты новые не просто так выходят. Используй auto, где это логично, используй structured bindings — это же красиво и надёжнее.
Вот, например, классика жанра, от которой у меня волосы дыбом встают:
// Было: риск утечки памяти при выбросе исключения. Это просто приглашение для бага.
void processOldStyle(Resource* res) {
res->acquire();
// ... код, который может выбросить исключение
res->release(); // До этой строчки можно и не дожить, если выше всё пойдёт по пизде.
}И после такого кода доверия ёбать ноль. А должно быть вот так:
// Стало: использование RAII. Умный указатель сам всё порешает, даже если всё горит.
void processModern() {
auto res = std::make_unique<Resource>(); // Создали, и забыли.
res->acquire();
// ... код
// release() вызовется в деструкторе Resource автоматически. Как по маслу.
}Видишь разницу? В первом случае — хитрая жопа, которая когда-нибудь, да выстрелит. Во втором — надёжно и без лишней головной боли.
Главное в ревью — не просто тыкнуть пальцем и сказать «фигово». Надо объяснить, почему фигово и как сделать хорошо. Ссылаться на стандарт, на гайдлайны, на лучшие практики. Чтобы человек не просто исправил, а понял и в следующий раз так не делал. А то иначе получается игра в «угадайку»: исправил, не угадал — отправляю обратно. Терпения ноль ёбать у всех кончится.
Короче, ревью — это не чтобы унизить, а чтобы помочь и проект от геморроя уберечь. Сделал хорошо — молодец. Накосячил — бывает, исправляем. Всем же лучше будет.