C++ Code Review: What Seniors Focus on First

Introduction: Why Does Your Review Turn into a Battlefield?

You submitted a Merge Request. Everything seems clean, the logic is correct, the code compiles. An hour later, you get a reply from a senior: "-10 points for Gryffindor" and 35 comments on 200 lines of code. Sound familiar?

C++ code review isn't just about checking syntax. It's an art that separates juniors from seniors. C++ gives the developer immense power over memory and performance, but with great power comes great responsibility for undefined behavior and resource leaks.

In this article, we'll get inside the head of an experienced reviewer. I've collected real comments that seniors leave on code review in production projects. If you write in C++ and want your PRs accepted on the first try, read carefully. And if you're a team lead, use this checklist for onboarding newcomers.

1. Shooting Yourself in the Foot: Manual Memory Management

The most common holy war in reviews is "why aren't you using smart pointers?". Seniors hate bare new and delete in modern C++ (C++11 and later).

Bad Example

class DataProcessor {\n    int* buffer;\n    size_t size;\npublic:\n    DataProcessor(size_t s) : size(s) {\n        buffer = new int[s]; // Who will delete this?\n    }\n\n    ~DataProcessor() {\n        delete[] buffer; // What if an exception is thrown in the constructor?\n    }\n};

Senior's comment: "RAII violation. If the constructor throws an exception after allocating memory, the destructor won't be called. Memory leak. Use std::vector or std::unique_ptr."

Correct Example

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

Seniors love RAII (Resource Acquisition Is Initialization). If you write new in your code, be prepared to explain why std::make_unique or std::vector aren't suitable. In 95% of cases, they are.

2. Const-correctness: Put a "Comma" on Yourself

A missing const is a red flag for any senior C++ developer. It's not just style; it's a contract with the code reader and the compiler.

What Annoys in Reviews

std::string getName() { // Why not const?\n    return name;\n}\n\nvoid printData(std::vector<int>& data) { // Copy? Reference? Mutating?\n    for (auto& item : data) {\n        std::cout << item;\n    }\n}

Senior's comment: "The getName() method doesn't modify the object — make it const. The data parameter isn't modified — pass it as const std::vector<int>&. Otherwise, the calling code doesn't know if you'll change its data or not."

Fixing It

std::string getName() const { // const method\n    return name;\n}\n\nvoid printData(const std::vector<int>& data) { // const reference\n    for (const auto& item : data) {\n        std::cout << item;\n    }\n}

The rule is simple: if a method doesn't change the object's state — const. If a function doesn't modify an argument — const T&. Const correctness in C++ is not an option; it's the standard.

3. Move Semantics: Copying is Not Our Religion

Modern C++ is about moving, not copying. Seniors immediately spot where you create unnecessary copies and kill performance.

Bad Pattern

std::vector<int> processData(std::vector<int> data) { // Copy on input\n    // ... lots of logic ...\n    return data; // Copy on output? Or NRVO?\n}\n\nvoid caller() {\n    std::vector<int> bigData = {1, 2, 3, ... 1000000};\n    auto result = processData(bigData); // Another copy!\n}

Comment