C++ Code Review | 20-Item "Checklist" [Essential for Professionals]

C++ Code Review | 20-Item "Checklist" [Essential for Professionals]

이 글의 핵심

C++ review checklist: RAII, data races, API contracts, and security pitfalls—actionable items for reviewers.

Memory Safety (5 Items)

1. Check for Memory Leaks

// ❌ Leak
void bad() {
    int* ptr = new int[100];
    // No delete[]!
}

// ✅ RAII
void good() {
    vector<int> v(100);  // Automatically released
}

2. Dangling Pointers

// ❌ Risky
int* bad() {
    int x = 10;
    return &x;  // Returning address of a local variable!
}

// ✅ Safe
int good() {
    int x = 10;
    return x;  // Return value
}

3. Double Delete

// ❌ Crash
int* ptr = new int(10);
delete ptr;
delete ptr;  // Double delete!

// ✅ Set to nullptr
int* ptr = new int(10);
delete ptr;
ptr = nullptr;
delete ptr;  // Safe

4. Array Out-of-Bounds Access

// ❌ Risky
int arr[10];
arr[10] = 5;  // Out-of-bounds access!

// ✅ Check
if (index < 10) {
    arr[index] = 5;
}

// ✅ Use vector
vector<int> v(10);
v.at(10) = 5;  // Throws exception

5. Use Smart Pointers

// ❌ Raw pointer
int* ptr = new int(10);
// ... complex logic ...
delete ptr;  // Easy to forget

// ✅ Smart pointer
auto ptr = make_unique<int>(10);
// Automatically released

Performance (5 Items)

6. Avoid Unnecessary Copies

// ❌ Copy
void process(vector<int> data) {  // Copy!
    // ...
}

// ✅ Use reference
void process(const vector<int>& data) {
    // ...
}

7. Use reserve

// ❌ Multiple reallocations
vector<int> v;
for (int i = 0; i < 1000; i++) {
    v.push_back(i);
}

// ✅ Pre-allocate
vector<int> v;
v.reserve(1000);
for (int i = 0; i < 1000; i++) {
    v.push_back(i);
}

8. Use Appropriate Data Structures

// ❌ Slow (O(n) search)
vector<int> v;
find(v.begin(), v.end(), x);

// ✅ Faster (O(log n) or O(1))
set<int> s;
s.find(x);

unordered_set<int> us;
us.find(x);

9. String Concatenation

// ❌ Slow
string result;
for (const auto& s : strings) {
    result += s;  // Reallocation every time
}

// ✅ Faster
ostringstream oss;
for (const auto& s : strings) {
    oss << s;
}
string result = oss.str();

10. Move Semantics

// ❌ Copy
vector<int> v1 = getData();
vector<int> v2 = v1;  // Copy

// ✅ Move
vector<int> v1 = getData();
vector<int> v2 = move(v1);  // Move

Readability (5 Items)

11. Clear Variable Names

// ❌ Unclear
int d;  // What does this mean?
int tmp;

// ✅ Clear
int daysUntilExpiry;
int userCount;

12. Function Length

// ❌ Too long (100+ lines)
void processData() {
    // ... 100 lines ...
}

// ✅ Split into smaller functions
void validateData() { /* ... */ }
void transformData() { /* ... */ }
void saveData() { /* ... */ }

void processData() {
    validateData();
    transformData();
    saveData();
}

13. Remove Magic Numbers

// ❌ Magic number
if (status == 2) {  // What is 2?
    // ...
}

// ✅ Use constants
const int STATUS_ACTIVE = 2;
if (status == STATUS_ACTIVE) {
    // ...
}

// ✅ Use enums
enum class Status { Inactive, Pending, Active };
if (status == Status::Active) {
    // ...
}

14. Comments

// ❌ Unnecessary comments
int x = 10;  // Assign 10 to x

// ❌ Outdated comments
// TODO: Fix this later (2020)

// ✅ Explain intent
// Set timeout to 10 seconds (considering server response time)
int timeout = 10;

15. Const Correctness

// ❌ No const
void print(vector<int>& v) {
    for (int x : v) {
        cout << x << " ";
    }
}

// ✅ Add const
void print(const vector<int>& v) {
    for (int x : v) {
        cout << x << " ";
    }
}

Safety (5 Items)

16. Exception Safety

// ❌ Leak on exception
void bad() {
    int* ptr = new int[100];
    riskyOperation();  // May throw exception
    delete[] ptr;  // Not executed!
}

// ✅ RAII
void good() {
    vector<int> v(100);
    riskyOperation();  // Safe even if exception occurs
}

17. nullptr Check

// ❌ No check
void process(int* ptr) {
    *ptr = 10;  // What if ptr is nullptr?
}

// ✅ Add check
void process(int* ptr) {
    if (!ptr) {
        throw invalid_argument("ptr is null");
    }
    *ptr = 10;
}

18. Integer Overflow

// ❌ Possible overflow
int a = INT_MAX;
int b = a + 1;  // Overflow!

// ✅ Add check
if (a > INT_MAX - 1) {
    throw overflow_error("overflow");
}
int b = a + 1;

19. Input Validation

// ❌ No validation
void setAge(int age) {
    this->age = age;  // Negative values allowed?
}

// ✅ Add validation
void setAge(int age) {
    if (age < 0 || age > 150) {
        throw invalid_argument("invalid age");
    }
    this->age = age;
}

20. Thread Safety

// ❌ Race condition
int counter = 0;

void increment() {
    counter++;  // Not thread-safe!
}

// ✅ Use mutex
mutex mtx;
int counter = 0;

void increment() {
    lock_guard<mutex> lock(mtx);
    counter++;
}

Code Review Process

1. Automated Checks

# Compile warnings
g++ -Wall -Wextra -Werror

# Static analysis
cppcheck --enable=all .
clang-tidy *.cpp

# Format check
clang-format -i *.cpp

2. Manual Checks

□ Does the code meet requirements?
□ Are tests sufficient?
□ Is error handling appropriate?
□ Are there performance issues?
□ Are there security vulnerabilities?
□ Is the code readable?
□ Is it documented?

3. Writing Feedback

✅ Good feedback:
"line 42: Using const reference for the vector can avoid copying."

❌ Bad feedback:
"This code is terrible."

Practical Example

Example 1: Code Before Review

void process(vector<int> data) {
    int* arr = new int[data.size()];
    
    for (int i = 0; i <= data.size(); i++) {
        arr[i] = data[i] * 2;
    }
    
    // ... processing ...
    
    delete arr;  // Not delete[]!
}

Issues:

  1. Unnecessary copy (vector)
  2. Use of raw pointer
  3. Out-of-bounds access (i <= size)
  4. delete vs delete[]

Example 2: Code After Review

void process(const vector<int>& data) {
    vector<int> result;
    result.reserve(data.size());
    
    for (int value : data) {
        result.push_back(value * 2);
    }
    
    // ... processing ...
}

Improvements:

  1. Removed copy by using const reference
  2. Used vector (RAII)
  3. Range-based for loop
  4. Improved performance with reserve

FAQ

Q1: How often should code reviews happen?

A: Ideally, for every PR/commit.

Q2: How many reviewers are needed?

A: At least 1, but 2 or more for critical code.

Q3: How long should a review take?

A: Around 1 hour for 200-400 lines of code.

Q4: What tools can be used for automation?

A:

  • clang-tidy
  • cppcheck
  • SonarQube
  • Coverity

Q5: What is a good code review culture?

A:

  • Provide constructive feedback
  • Focus on improving code, not criticizing the author
  • Treat it as a learning opportunity

Q6: How to create a review checklist?

A:

  1. Analyze past bugs in the team
  2. Identify common mistakes
  3. Include coding style guidelines

같이 보면 좋은 글 (내부 링크)

이 주제와 연결되는 다른 글입니다.

  • C++ RAII 패턴 | “리소스 관리” 완벽 가이드
  • C++ const 완벽 가이드 | “const 정확성” 실전 활용
  • C++ 스마트 포인터 | unique_ptr/shared_ptr “메모리 안전” 가이드


이 글에서 다루는 키워드 (관련 검색어)

C++, Code Review, Checklist, Code Quality, Professional 등으로 검색하시면 이 글이 도움이 됩니다.