Module 9 — Misconceptions and Mistakes¶

Two different problems, fixed two different ways. A misconception is a wrong mental model, and only evidence removes it. A mistake is a concrete error, and information fixes it.

Every compiler message on this page was captured by compiling the snippet shown with g++ -std=c++17 -Wall -Wextra on the toolchain this course is verified against.

Four of this module's failure modes produce no diagnostic at all, and they are the reason this module exists.

Part 1 — Misconceptions¶

"A member is just a field, and holding a pointer means I have the object"¶

Why this feels true. All four member forms look identical at the point of use: logger.write(...) works whether logger is a value, a pointer dereferenced, or a reference.

Where it breaks. The forms differ in exactly the questions a program eventually asks:

  value member      owns it; copied, moved and destroyed with the owner
  pointer member    borrows it; may be null, may be reseated, never deletes
  reference member  borrows it; never null, never reseated, and the class
                    loses its assignment operator

A pointer member does not mean "I have the object". It means "I know where one was at some point", with no guarantee that it is still alive — which is Module 11's subject, and the reason std::unique_ptr and std::shared_ptr exist.

The correct model. A member declaration is a statement about lifetime: who creates it, who destroys it, whether it can be absent, whether it can change. Write those four answers down for every member, and prefer the weakest form that is true.

The re-test. class Session { Logger& logger_; }; — can a Session be assigned? Can the Logger it refers to outlive the Session? Which of those two is a bug?

"Returning a reference is always safer than returning a copy"¶

Why this feels true. Copying is expensive, and a reference avoids the copy — and const T& parameters are the standard advice from Module 9's own examples.

Where it breaks. examples/module-09/04-…/main.cpp breaks both of a class's invariants with one accessor:

  before:  size 3/3, every student has non-negative credits
  after:   size 5/3, and at(0) has -104 credits
    - the capacity is exceeded (5 of 3);
    - a student exists with a negative credit count;
    - totalCredits() is now wrong for every caller, forever.

The correct model. A non-const reference to a member is a transfer of control, not a performance decision. const& is read-only and safe for invariants, but it still binds every caller to the container's type. snapshot() is the form that returns something safe to keep — at the cost of a copy and of staleness. The three costs are different, and the right answer depends on which one the caller was going to pay anyway.

The re-test. A caller needs the students in sorted order and then needs to count them. Which form do you offer, and why does the answer change if the caller also needs to keep the list after the roster is gone?

"A forward declaration is enough to use a class"¶

Why this feels true. It compiles for pointers and references, and it is the standard answer to include cycles, so it reads like a general fix.

Where it breaks. Two places, with different messages.

A value member needs the complete type:

error: field 'item_' has incomplete type 'Item'
      |          ^~~~~
note: forward declaration of 'class Item'
      | class Item;                      // forward declaration: the name, not the layout
      |       ^~~~

Calling a member function through a reference or pointer needs the definition too:

error: invalid use of incomplete type 'const class MemberD'
      |           ^~~~~~
note: forward declaration of 'class MemberD'

The correct model. A forward declaration says "there is a class with this name". The compiler needs that much for a pointer or reference member; it needs the layout for a value member, and the member list to call a function. So: declare in the header, define in the .cpp — which is where the include belongs.

The re-test. A header declares class B; and a member B b_;. Name the error, then name the two legitimate fixes and the different problem each one introduces.

"An include cycle is a compiler quirk I can fix by adding includes"¶

Why this feels true. The message names a type you clearly included, and adding more includes is the obvious response. It makes the error worse.

Where it breaks. This is the whole output of a two-header cycle, and the first two lines are the ones that explain it:

In file included from cycle-member.h:2,
                 from cycle-main.cpp:1:
cycle-loan.h: In member function 'void Loan::assignTo(const Member&)':
cycle-loan.h:15:49: error: invalid use of incomplete type 'const class Member'
cycle-loan.h:6:7: note: forward declaration of 'class Member'

cycle-main.cpp includes cycle-member.h, which includes cycle-loan.h, which includes cycle-member.h — whose include guard has already fired, so its contents are skipped. Loan sees the forward declaration and nothing else.

The correct model. One of the two headers must stop including the other. Keep the declaration that needs only a name (the pointer or reference member) in the header, and move the function that needs the definition into a .cpp that includes both. Read In file included from lines as a stack trace: they show you the cycle.

The re-test. Two classes hold each other by reference. Which header includes what, and where does the #include of the second header go?

"A reference member is the safest kind of member"¶

Why this feels true. It cannot be null, it cannot be reseated, and it removes a whole class of null checks. What is not to like?

Where it breaks. The class stops being assignable, and the compiler says so immediately:

error: use of deleted function 'TickerA& TickerA::operator=(const TickerA&)'
note: 'TickerA& TickerA::operator=(const TickerA&)' is implicitly deleted because the
      default definition would be ill-formed:
error: non-static reference member 'LogA& TickerA::logger_', cannot use default
       assignment operator

and it cannot be initialised from a temporary:

error: cannot bind non-const lvalue reference of type 'LogB&' to an rvalue of type 'LogB'
note:   initializing argument 1 of 'SessionB::SessionB(LogB&)'

The correct model. Both errors are the language refusing to let you lie. Assignment means "become equal to that object", and a reference cannot be made to refer elsewhere; binding to a temporary would create a member referring to an object that dies at the end of the statement. Neither is an obstacle — they are the price of "this relationship is fixed for life", and if that is not what you mean, a pointer or a value member says what you do mean.

The re-test. A class holds const Logger&. Which of construction, copy construction and assignment are available, and which would you expect to be missing?

"Inheriting from the container is the quickest way to get its storage"¶

Why this feels true. It is quicker — one line instead of five forwarders — and the storage, the size and the ordering all arrive for free.

Where it breaks. Three independent reasons, all fatal:

  1. std::deque has no virtual destructor, so `delete` through a `std::deque*`
     is undefined behaviour (Module 7);
  2. push_front, insert, erase, clear, resize, operator[] would all be public
     on a type whose job is arrival order;
  3. every caller would depend on std::deque, so replacing it later would be a
     change to every call site.

The third is the one that hurts six months later: the composed version in examples/module-09/05-extension-by-composition/ compiles unchanged against two different containers, and the diff between the two builds touches only the lines that print the storage name.

The correct model. Extension by composition: hold the container privately, publish the few operations callers need as forwarders, and put the invariant where the mutation happens. The forwarders are the cost, and they are cheap. See Effective C++, Item 38.

The re-test. You need a std::map plus an extra operation that counts lookups. Write the two class declarations you would consider, and say which three questions decide it.

"Enforcing the rule in the mutator is enough"¶

Why this feels true. add() checks the capacity and the credit count, and it is the only function that takes a new student. There is no way to bypass it — unless the class publishes something else that changes the contents.

Where it breaks. It published std::vector<Student>&: two push_back calls and a front().addCredits(-200) later, both rules are gone. The same defect appears in the queue example in a subtler form: an inherited push_front or clear would desynchronise lateCount_ without breaking any rule the class states, producing a wrong count rather than an obviously invalid object.

The correct model. An invariant is only as strong as the set of ways to change the object, not the set of ways you remembered. Count them: constructors, mutators, friend functions, and anything that hands out a reference to state. The class must be able to enumerate every door, or it cannot know the rule still holds.

The re-test. A class has add(), remove(), a copy assignment, and a friend function. How many ways are there to change its collection, and what does each one have to maintain?

Part 2 — Common programming mistakes¶

Symptom	Cause	Fix
`error: use of deleted function` on assignment	A reference member (or `const` member) deleted the implicit assignment operator	Decide what assignment should mean: write one, or make the class non-assignable deliberately
`error: cannot bind non-const lvalue reference of type 'T&' to an rvalue`	A reference member was initialised from a temporary	Keep a value member, or make the referred-to object outlive the class and say so
`error: passing 'const X' as 'this' argument discards qualifiers`	A `const` object called a non-`const` member function	Add a `const` overload; never `const_cast` it away
`error: field 'x' has incomplete type 'X'`	A value member declared from a forward declaration	Include the header where the member is defined
`error: invalid use of incomplete type 'X'`	Calling a member function through a forward-declared type	Move the function body into a `.cpp` that includes the header
`error: 'X' does not name a type`, in a file that includes `X`'s header	An include cycle, with the guard already fired	Break the cycle: one header stops including the other; read the `In file included from` stack
A member is used while it is still empty/zero	It is declared after something that refers to or reads it	Declare referenced members first; the compiler's `-Wreorder` covers initialiser lists, not bodies
Callers mutate state you meant to protect	A non-`const` accessor returned a reference to a member	Return `const&`, an iterator pair, a snapshot, or a callback-based query
Two objects both delete the same pointer	Two owners were created by a copy, or by a design with no owner	One owner. Module 11 makes this mechanical
A non-owning member points at something already destroyed	The owner's lifetime ended first	Own it, or document the ordering requirement, or use a `weak_ptr` (Module 11)

Mistake 1 — the accessor that hands out the container [L3 Modify]¶

Reproduce it.

class Roster {
public:
    std::vector<Student>& students() { return students_; }     // ← the door
private:
    std::size_t capacity_;
    std::vector<Student> students_;
};

void use(Roster& roster) {
    roster.students().push_back(Student("Farhan", 120));
    roster.students().front().addCredits(-200);
}

What you see. Nothing at compile time. At run time, the capacity is exceeded and a student has negative credits — and the program's own output says which functions are now wrong:

    - totalCredits() is now wrong for every caller, forever.

Why. The class's rules are enforced in add(). students() is a second, unguarded way to change the same state — and it is the one callers will use, because it is the most flexible.

Fix it. Remove the accessor and offer a capability per caller need:

    const Student& at(std::size_t index) const;
    std::vector<Student>::const_iterator begin() const;
    std::vector<Student>::const_iterator end() const;
    std::vector<Student> snapshot() const;
    void forEach(const std::function<void(const Student&)>& visit) const;

Prevent it. Grep your public interface for & returns whose type is a container. For each one, ask what the caller could do with it that the class cannot express — and if the answer is "add, remove or reorder", the accessor is a bug.

Mistake 2 — a reference member by accident [L3 Modify]¶

Reproduce it.

class TickerA {
public:
    explicit TickerA(LogA& logger) : logger_(logger) {}
private:
    LogA& logger_;
};

void use() {
    LogA a;
    TickerA first(a);
    TickerA second(a);
    first = second;                 // ← the class is not assignable
}

What you see.

error: use of deleted function 'TickerA& TickerA::operator=(const TickerA&)'
note: 'TickerA& TickerA::operator=(const TickerA&)' is implicitly deleted because the
      default definition would be ill-formed:
error: non-static reference member 'LogA& TickerA::logger_', cannot use default
       assignment operator

Why. Assignment would have to make first.logger_ refer to second.logger_'s target. A reference cannot be reseated, so the compiler deletes the operator instead of choosing a behaviour for you.

Fix it. Choose deliberately:

class Ticker {
public:
    Ticker& operator=(const Ticker& other) {                 // logger stays, label changes
        if (this != &other) {
            label_ = other.label_;
        }
        return *this;
    }
private:
    LogA& logger_;                  // still not reseated
    std::string label_;
};

or use a pointer member, which assignment copies naturally — and then answer the null question the pointer creates.

Prevent it. When you write a reference member, write the sentence that says the relationship is fixed for the object's whole life. If the class needs to be assignable, the relationship is not fixed, and the member is the wrong form.

Mistake 3 — the forward declaration that was not enough [L3 Modify]¶

Reproduce it.

class Item;                     // forward declaration: the name, not the layout

class Shelf {
    Item item_;                 // a value member needs the complete type
};

What you see.

error: field 'item_' has incomplete type 'Item'
note: forward declaration of 'class Item'

and, for the same mistake at a call site rather than a declaration:

error: invalid use of incomplete type 'const class MemberD'
note: forward declaration of 'class MemberD'

Why. A value member's size and layout are part of the class's own layout, so the compiler must know them. A member call needs the member list. A pointer or reference needs neither — and that is exactly why the forward declaration is the tool for breaking cycles.

Fix it. Include the header where the value member is declared; for the cycle case, move the implementation into a .cpp and include both headers there.

Prevent it. Two conventions that prevent both errors: headers include only what they store or inherit; everything else is a forward declaration in the header and an include in the source file.

Mistake 4 — the cycle that "cannot be a cycle" [L4 Analyze]¶

Reproduce it. Two headers that include each other, and a program that includes one of them:

// cycle-member.h
#pragma once
#include "cycle-loan.h"
class Loan;
class Member { void hold(const Loan& loan); … };

// cycle-loan.h
#pragma once
#include "cycle-member.h"
class Member;
class Loan { void assignTo(const Member& member) { (void)member.name(); } … };

What you see.

In file included from cycle-member.h:2,
                 from cycle-main.cpp:1:
cycle-loan.h: In member function 'void Loan::assignTo(const Member&)':
cycle-loan.h:15:49: error: invalid use of incomplete type 'const class Member'
cycle-loan.h:6:7: note: forward declaration of 'class Member'

Why. The In file included from lines are the cycle, printed as a stack: cycle-main.cpp → cycle-member.h → cycle-loan.h → (back to) cycle-member.h, whose guard has already fired. On the second visit the header contributes nothing, so Member is only ever a forward declaration in cycle-loan.h.

Fix it. Delete one of the includes. cycle-loan.h keeps class Member; and declares void assignTo(const Member&);; cycle-loan.cpp includes cycle-member.h and defines it. Each header now includes only what it needs, and the definition sees both types.

Prevent it. Adopt the rule no header includes a header that includes it, and read the In file included from block before you read the error — it names the cycle, and it is usually the whole diagnosis.

Mistake 5 — the invariant with a second door [L4 Analyze]¶

Reproduce it. A queue that counts late submissions, where the count is maintained in push and pop — and where the storage is also reachable another way:

class DeadlineQueue : public std::deque<Submission> {     // the second door, times six
public:
    void push(…);          // ++lateCount_ if late
    bool pop();            // --lateCount_ if late
private:
    std::size_t lateCount_ = 0;
};

What you see. No diagnostic. Then, months later, a count that is wrong:

  queue.clear()          -> lateCount_ untouched, contents gone
  queue.pop_front()      -> an element removed, count untouched
  queue.push_back(late)  -> an element added, count untouched
  queue.erase(it)        -> count untouched

Why. The class states an invariant and enforces it in two functions, while the inherited interface offers at least four more ways to change the contents. Each of them is a line of code that looks correct and produces a wrong count — the worst kind of bug, because the object still looks valid.

Fix it. Compose instead of inheriting, and publish the six operations callers actually need:

class DeadlineQueue {
public:
    void push(std::string student, std::string assignment, bool late);
    bool pop();
    const Submission& front() const;
    std::size_t size() const;
    std::size_t lateCount() const;
    bool empty() const;
private:
    Storage items_;
    std::size_t lateCount_ = 0;
};

Prevent it. When you state an invariant, list every operation that could break it — not every operation you wrote. If the class inherits, the list is not yours to know, which is itself the answer.

Next: Debugging — the ownership map, and a diagnosis that begins by asking who was supposed to delete it.