본문으로 건너뛰기 C++ Code Review | 20-Item 'Checklist' [Essential for Prof...

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

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

이 글의 핵심

C++ code review checklist. Memory safety, performance, readability, and security checks.

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

Other articles related to this topic.



Keywords Covered in This Article (Related Search Terms)

This article covers C++, Code Review, Checklist, Code Quality, Professional.