Chapter 8: Best Practices and Common Pitfalls

This chapter covers the wisdom that comes from experience: what works, what doesn't, and why.

Core Guidelines

The C++ Core Guidelines by Bjarne Stroustrup and Herb Sutter are the official reference. Here are the most important principles:

Memory Safety

Never Use Raw new/delete

// BAD
void bad() {
    Widget* w = new Widget();
    // ... what if exception is thrown?
    delete w;  // Might never be reached
}

// GOOD
void good() {
    auto w = std::make_unique<Widget>();
    // Automatically cleaned up, even with exceptions
}

// If you absolutely need raw pointers (rare)
void observer(Widget* w) {  // Non-owning
    // Use but don't delete
}

Prefer Stack Over Heap

// UNNECESSARY heap allocation
auto p = std::make_unique<std::string>("hello");
process(*p);

// BETTER: stack allocation
std::string s = "hello";
process(s);

Initialize All Variables

// BAD: uninitialized
int x;           // Contains garbage
int* p;          // Points to garbage

// GOOD: always initialize
int x = 0;
int y{};         // Value-initialized (0)
int* p = nullptr;

// Use brace initialization to catch narrowing
int a = 3.14;   // Silently truncates to 3
int b{3.14};    // Compiler error!

Avoid Pointer Arithmetic

// BAD: manual indexing
void process(int* arr, size_t size) {
    for (size_t i = 0; i < size; ++i) {
        *(arr + i) = 0;  // Error-prone
    }
}

// GOOD: use containers
void process(std::vector<int>& arr) {
    for (int& x : arr) {
        x = 0;
    }
}

// Or std::span (C++20)
void process(std::span<int> arr) {
    for (int& x : arr) {
        x = 0;
    }
}

Resource Management

Follow RAII

// BAD: manual resource management
void bad() {
    FILE* f = fopen("data.txt", "r");
    if (!f) return;  // OOPS: nothing to clean up here, but...

    if (someCondition) {
        fclose(f);  // Must remember
        return;
    }

    // What if exception is thrown here?

    fclose(f);  // Must remember again
}

// GOOD: RAII wrapper
class File {
    FILE* f;
public:
    File(const char* name) : f(fopen(name, "r")) {
        if (!f) throw std::runtime_error("Failed to open");
    }
    ~File() { if (f) fclose(f); }

    // Delete copy, allow move
    File(const File&) = delete;
    File& operator=(const File&) = delete;
    File(File&& other) noexcept : f(other.f) { other.f = nullptr; }
    File& operator=(File&& other) noexcept {
        std::swap(f, other.f);
        return *this;
    }

    FILE* get() { return f; }
};

void good() {
    File f("data.txt");  // Opens
    // Use f...
}  // Automatically closed, even on exception

Use Standard RAII Types

// Instead of custom classes, prefer:
std::unique_ptr<T>      // Unique ownership
std::shared_ptr<T>      // Shared ownership
std::lock_guard<T>      // Mutex locking
std::unique_lock<T>     // Flexible mutex locking
std::fstream            // File I/O
std::vector<T>          // Dynamic arrays

Function Design

Prefer Return Values Over Output Parameters

// BAD: output parameters
void getResult(int& out_value, std::string& out_error);

// GOOD: return values
int getResult();  // Simple case

// GOOD: return struct for multiple values
struct Result {
    int value;
    std::string error;
};
Result getResult();

// GOOD: std::optional for "might not exist"
std::optional<int> findValue(const std::string& key);

// GOOD: std::expected for errors (C++23)
std::expected<int, std::string> compute();

Pass Parameters Correctly

// Small types: pass by value
void process(int x);
void process(double x);
void process(std::string_view sv);  // View is cheap

// Large types: pass by const reference for read
void process(const std::vector<int>& data);
void process(const std::string& text);

// Modify caller's value: pass by reference
void modify(std::vector<int>& data);

// Transfer ownership: pass by value and move
void takeOwnership(std::unique_ptr<Widget> w);
void takeOwnership(std::string s);  // Caller can move or copy

// Sink parameter pattern
class Container {
public:
    void add(std::string item) {  // By value
        items.push_back(std::move(item));  // Move into container
    }
private:
    std::vector<std::string> items;
};

// Usage:
container.add("literal");           // Constructs in place
container.add(existingString);      // Copies
container.add(std::move(tempStr));  // Moves

Use const Liberally

class Widget {
    int value;

public:
    // const member function: doesn't modify object
    int getValue() const { return value; }

    // Non-const: may modify
    void setValue(int v) { value = v; }
};

void process(const std::vector<int>& data) {
    // data[0] = 42;  // ERROR: const
    int x = data[0];  // OK: reading
}

// const at end of declaration = top-level const
const int* p1 = &x;      // Pointer to const int (can change p1)
int* const p2 = &x;      // Const pointer to int (can't change p2)
const int* const p3 = &x; // Const pointer to const int

Class Design

Make Interfaces Hard to Misuse

// BAD: easy to swap arguments
void setDimensions(int width, int height);
setDimensions(100, 50);   // Is this width=100, height=50?
setDimensions(50, 100);   // Or this?

// GOOD: strong types
struct Width { int value; };
struct Height { int value; };
void setDimensions(Width w, Height h);
setDimensions(Width{100}, Height{50});  // Clear!

// Or use named parameters pattern
class WidgetBuilder {
public:
    WidgetBuilder& width(int w) { w_ = w; return *this; }
    WidgetBuilder& height(int h) { h_ = h; return *this; }
    Widget build();
private:
    int w_, h_;
};
auto widget = WidgetBuilder().width(100).height(50).build();

Default to =delete Over Empty Implementations

class NonCopyable {
public:
    NonCopyable() = default;

    // Explicitly delete unwanted operations
    NonCopyable(const NonCopyable&) = delete;
    NonCopyable& operator=(const NonCopyable&) = delete;

    // Allow move if appropriate
    NonCopyable(NonCopyable&&) = default;
    NonCopyable& operator=(NonCopyable&&) = default;
};

Use override and final

class Base {
public:
    virtual void process();
    virtual void compute() const;
    virtual ~Base() = default;
};

class Derived : public Base {
public:
    void process() override;  // Catches typos!
    // void proccess() override;  // ERROR: no Base::proccess
    // void compute() override;   // ERROR: missing const

    void compute() const final;  // No further override allowed
};

Prefer Composition Over Inheritance

// QUESTIONABLE: inheritance for code reuse
class Stack : public std::vector<int> {  // "is-a" vector?
    // Exposes all vector methods (push_back, insert, etc.)
};

// BETTER: composition
class Stack {
    std::vector<int> data;  // "has-a" vector
public:
    void push(int x) { data.push_back(x); }
    int pop() {
        int x = data.back();
        data.pop_back();
        return x;
    }
    bool empty() const { return data.empty(); }
};

Error Handling

Use Exceptions for Exceptional Conditions

// BAD: error codes everywhere
int compute(int x, int& result) {
    if (x < 0) return -1;  // Error code
    result = x * 2;
    return 0;  // Success
}

int r;
if (compute(x, r) != 0) {
    // Handle error
}

// GOOD: exceptions for errors
int compute(int x) {
    if (x < 0) {
        throw std::invalid_argument("x must be non-negative");
    }
    return x * 2;
}

try {
    int r = compute(x);
} catch (const std::invalid_argument& e) {
    // Handle specific error
} catch (const std::exception& e) {
    // Handle any standard exception
}

noexcept for Move Operations

class Buffer {
public:
    // Move operations should be noexcept for optimization
    Buffer(Buffer&& other) noexcept
        : data(other.data), size(other.size) {
        other.data = nullptr;
        other.size = 0;
    }

    Buffer& operator=(Buffer&& other) noexcept {
        if (this != &other) {
            delete[] data;
            data = other.data;
            size = other.size;
            other.data = nullptr;
            other.size = 0;
        }
        return *this;
    }

private:
    int* data;
    size_t size;
};

Use std::optional for "Might Not Exist"

// BAD: magic values
int findIndex(const std::vector<int>& v, int target) {
    for (size_t i = 0; i < v.size(); ++i) {
        if (v[i] == target) return i;
    }
    return -1;  // Magic value meaning "not found"
}

// GOOD: explicit optionality
std::optional<size_t> findIndex(const std::vector<int>& v, int target) {
    for (size_t i = 0; i < v.size(); ++i) {
        if (v[i] == target) return i;
    }
    return std::nullopt;
}

if (auto idx = findIndex(vec, 42)) {
    std::cout << "Found at " << *idx << "\n";
}

Common Pitfalls

Object Slicing

class Base {
public:
    virtual std::string name() const { return "Base"; }
};

class Derived : public Base {
public:
    std::string name() const override { return "Derived"; }
};

// SLICING: Derived part is cut off
void process(Base b) {  // Pass by value!
    std::cout << b.name();  // Always "Base"
}

Derived d;
process(d);  // Sliced to Base

// FIX: pass by reference or pointer
void process(const Base& b) {
    std::cout << b.name();  // "Derived" for Derived objects
}

Dangling References

// BAD: returning reference to local
const std::string& bad() {
    std::string local = "hello";
    return local;  // DANGLING: local is destroyed
}

// BAD: reference to temporary in range-for
for (const auto& x : getVector()) {  // If getVector() returns by value...
    // Vector is destroyed, x is dangling!
}

// FIX: copy or store
std::vector<int> vec = getVector();
for (const auto& x : vec) {
    // Safe: vec lives through loop
}

Iterator Invalidation

std::vector<int> v = {1, 2, 3, 4, 5};

// BAD: modifying during iteration
for (auto it = v.begin(); it != v.end(); ++it) {
    if (*it == 3) {
        v.erase(it);  // Iterator invalidated!
    }
}

// GOOD: use erase-remove idiom
v.erase(std::remove(v.begin(), v.end(), 3), v.end());

// GOOD: C++20
std::erase(v, 3);

Most Vexing Parse

// Intend to create default Widget
Widget w();  // OOPS: declares a function!

// Fixes:
Widget w;      // Default construction
Widget w{};    // Brace initialization (C++11)
auto w = Widget();  // Copy initialization

Undefined Behavior

// Common UB sources:
int* p = nullptr;
*p = 42;              // UB: null dereference

int arr[5];
arr[10] = 0;          // UB: out of bounds

int x;
int y = x + 1;        // UB: uninitialized read

int a = INT_MAX;
int b = a + 1;        // UB: signed overflow

// Detect with sanitizers:
// g++ -fsanitize=undefined,address ...

String Lifetime Issues

// BAD: dangling string_view
std::string_view bad() {
    std::string s = "temporary";
    return s;  // s destroyed, view is dangling
}

// BAD: dangling c_str()
const char* alsoBad() {
    std::string s = "temporary";
    return s.c_str();  // s destroyed, pointer invalid
}

// GOOD: return by value
std::string good() {
    std::string s = "temporary";
    return s;  // Moved efficiently
}

Performance Tips

Reserve Vector Capacity

// BAD: multiple reallocations
std::vector<int> v;
for (int i = 0; i < 10000; ++i) {
    v.push_back(i);  // May reallocate many times
}

// GOOD: reserve upfront
std::vector<int> v;
v.reserve(10000);  // One allocation
for (int i = 0; i < 10000; ++i) {
    v.push_back(i);  // No reallocations
}

Prefer emplace Over push

std::vector<std::pair<int, std::string>> v;

// Creates temporary, then moves
v.push_back(std::make_pair(1, "hello"));

// Constructs in place (no temporary)
v.emplace_back(1, "hello");

// Same for maps
std::map<int, std::string> m;
m.emplace(1, "hello");  // Better than m[1] = "hello" or m.insert()

Move When Done

void process(std::vector<int> data) {
    // ... uses data ...
}

std::vector<int> myData = getData();
// ... use myData ...

// If done with myData, move it
process(std::move(myData));
// Don't use myData after this

Avoid Copies in Loops

// BAD: copies on each iteration
for (auto pair : map) {  // Copies each pair
    process(pair.second);
}

// GOOD: reference
for (const auto& pair : map) {
    process(pair.second);
}

// GOOD: structured binding with reference
for (const auto& [key, value] : map) {
    process(value);
}

Code Style and Naming

// Naming conventions (Google style, widely adopted):

// Types: CamelCase
class MyClass {};
struct MyStruct {};
enum class Color { Red, Green, Blue };

// Functions: camelCase or snake_case (pick one, be consistent)
void doSomething();
int calculate_value();

// Variables: camelCase or snake_case
int itemCount;
std::string user_name;

// Constants: kCamelCase or ALL_CAPS
constexpr int kMaxSize = 100;
constexpr int MAX_SIZE = 100;

// Member variables: trailing underscore or m_ prefix
class Widget {
    int value_;      // Trailing underscore
    int m_count;     // m_ prefix (older style)
};

// Namespaces: lowercase
namespace my_project {
namespace detail {
}
}

// Macros: ALL_CAPS (avoid macros when possible)
#define MAX_BUFFER_SIZE 1024

Exercises

  1. Memory Safety Audit

    • Review code for raw new/delete
    • Convert to smart pointers
    • Check for dangling references
  2. Exception Safety

    • Write a function that could throw
    • Ensure resources are cleaned up
    • Add noexcept where appropriate
  3. Performance Review

    • Profile code for unnecessary copies
    • Apply reserve, emplace, move
    • Measure improvement
  4. Code Review Checklist

    • Create a checklist of these best practices
    • Review your own code against it

Previous: 07 - Build Systems Next: 09 - Practical Projects