Code review in C++: what to check and how to give feedback in practice

Code review in C++: what to check and how to give feedback in practice

Code review is not just a formality, but a powerful tool for improving code quality. In C++, review is especially important due to the complexity of the language: manual memory management, templates, multiple inheritance. In this article, we will break down what to pay attention to when reviewing C++ code and how to formulate comments so that they are helpful, not offensive.

1. Memory safety and RAII

The first thing to check is resource management. C++ has no garbage collector, so memory leaks and double deletion are common problems. The key pattern is RAII (Resource Acquisition Is Initialization).

What to check:

\n
• Are smart pointers (std::unique_ptr, std::shared_ptr) used instead of raw new/delete?
\n
• Is there any manual call to delete in the code? (Except for rare low-level cases)
\n
• Are RAII classes implemented correctly: resource acquired in the constructor and released in the destructor?
\n
• Is the virtual destructor in the base class forgotten?
\n

// ❌ Bad: manual management\nvoid process() {\n    int* data = new int[100];\n    // ... if an exception occurs, memory will leak\n    delete[] data;\n}\n\n// ✅ Good: RAII via std::vector\nvoid process() {\n    std::vector<int> data(100);\n    // safe with exceptions\n}

How to give feedback: Instead of "You forgot to delete memory," say: "It's better to use std::vector or std::unique_ptr here — this will automatically free resources and make the code exception-safe."

2. Const-correctness and references

C++ allows you to explicitly specify that data should not be changed. This improves readability and allows the compiler to better optimize the code.

What to check:

\n
• Are all parameters that are not modified marked as const?
\n
• Are large objects passed by const reference (const T&), rather than by value?
\n
• Are methods that do not modify the object marked as const?
\n
• Is const_cast used without absolute necessity?
\n

// ❌ Bad: copying a large object\nstd::string toUpper(std::string s); // copy\n\n// ✅ Good: const reference\nstd::string toUpper(const std::string& s); // no copy\n\n// ❌ Bad: method is not const, although it doesn't change state\nclass Point {\n    int x, y;\npublic:\n    int getX() { return x; } // should be const\n};\n\n// ✅ Good\nint getX() const { return x; }

How to give feedback: "Let's add const for parameters that are not modified — this will make the interface clearer and prevent accidental changes."

3. Using STL and algorithms

The C++ standard library contains many ready-made solutions. Reinventing the wheel is a common mistake for beginners.

What to check:

\n
• Are there manual loops that could be replaced with algorithms (std::find, std::sort, std::transform)?
\n
• Is the container chosen correctly? (std::vector for random access, std::list for frequent insertions in the middle)
\n
• Is std::string_view used for passing strings without copying?
\n
• Is reserve() forgotten for std::vector when the size is known?
\n

// ❌ Bad: manual search\nbool contains(const std::vector<int>& v, int val) {\n    for (size_t i = 0; i < v.size(); ++i) {\n        if (v[i] == val) return true;\n    }\n    return false;\n}\n\n// ✅ Good: using STL\nbool contains(c