Module 16 — Misconceptions and Mistakes¶

Part 1 — Misconceptions¶

1. "More files means a better structure."¶

Splitting looks organised, and every tutorial project with a src/, include/ and utils/ folder looks more professional than one long file. But the measure is not the number of files: it is whether a reader can name what each one is for. Two files split along the wrong boundary — helpers.cpp holding dates and strings, stuff.cpp holding the rest — are harder to read than one honest file, because the reader has to hold both and reconstruct the whole to find anything.

The boundary to split on is a responsibility: a name you would say out loud to a colleague.

2. "If it is in a header, it is available everywhere."¶

Headers are included everywhere, so it feels as though defining something in one puts it in the project's vocabulary. What actually happens is the opposite: the definition is duplicated into every translation unit that includes the header, and the link step fails. In the worked example the same int shelf_capacity() was defined in two objects, and the entire linker message was:

collect2.exe: error: ld returned 1 exit status

Nothing about headers, nothing about functions. The rule: a header declares; a source file defines — unless the function is inline, in which case a definition per translation unit is exactly what the standard permits.

3. "`static` and an unnamed namespace are the same thing."¶

They give the same effect at namespace scope — internal linkage — and for a single function it makes little practical difference. The unnamed namespace is the idiomatic C++ choice because it also works for types and for whole groups of declarations:

namespace {
struct Session { /* ... */ };   // this type is invisible outside this file
int scale(int value) { return value * 2; }
}

The misconception that costs marks is what internal linkage means. A name with internal linkage cannot be linked from another file — so a matching declaration in a header is a promise nobody can keep, and the only diagnostic you will get is an unused-function warning that names the symbol:

helper.cpp:8:5: error: 'int {anonymous}::scale(int)' defined but not used [-Werror=unused-function]

4. "File I/O usually works, so checking is paranoia."¶

It has worked every time so far, which is precisely the danger: the failure is silent and produces a plausible result rather than an error.

A. unchecked open : loaded 0 records. No message, no crash, and the program believes the catalogue is empty.
B. checked open   : refusing to continue - oop-course-stream-state.txt could not be opened

Line A looks like a working program with no data. Nothing in the program's output distinguishes "the file was empty" from "the file was never opened", and a user reporting the bug will say the program lost their records — which is true, and took no exception to accomplish.

5. "A file format is trivial — I will just write the fields."¶

Writing is the easy half. Reading is where the format is decided, because the reader has to cope with everything the writer never imagined: a separator inside the data, a field removed by hand, a blank line, a file from an older version, a record whose number of fields is wrong.

7. truncated record        : rejected - line 3: malformed record 'Compilers|Aho'
8. corrupted field         : rejected - line 3: malformed record 'Compilers|Aho|twenty thirteen'

A format is a contract, and it needs the three things every contract needs: a version marker, an escaping rule for the separator, and a documented behaviour when a record does not satisfy it.

6. "`std::getline` and `>>` can be mixed freely."¶

Both read input, so they look interchangeable. They disagree about the delimiter, and the disagreement is silent:

D. >> then getline: number 42, rest [] - the newline is still in the stream
   after ignore()  : number 42, rest [second line]

>> stops before the newline; getline consumes it as part of the line. Mixed without ignore(std::numeric_limits<std::streamsize>::max(), '\n'), every subsequent getline returns an empty string — and the usual reaction is to "fix" it by reading with >> everywhere, which breaks titles containing spaces.

7. "A test that prints the right value is a test."¶

It showed the correct output, and the output was correct. But a printing test compares nothing: it produces evidence for a human and checks nothing itself. Run against an implementation whose only fault is a cap one day too generous, it printed the same number:

  correct implementation:
    fine for 10 overdue days (staff): 15
  implementation with a cap one day too generous:
    fine for 10 overdue days (staff): 15

A test is a computation that fails when a value is wrong. Forty lines of check(bool, const std::string&) and a failure counter is a complete test framework for this course's purposes.

8. "Tests slow down development."¶

Writing them takes time, and the hours are visible; the time they save is invisible because the disasters did not happen. The honest answer is the one the module's own procedure rests on: without a green suite, refactoring working code is a bet, and a design nobody dares change stops improving. The capstone asks you to refactor — that is why the tests are not optional.

9. "The design report is paperwork."¶

It does not affect the program, and it may not be marked by anyone. But the code alone demonstrates level 6, and the exit level of this module is 8: the evaluation is the assessment. Writing down the decision you would reverse is also the only reliable way to find out whether you understand your own design — people who cannot name a weakness usually cannot name a trade-off either.

10. "A finished system has no weaknesses left."¶

Finished implies complete, and the report has to name one. Every system of this size has one, and the reason is structural rather than a failure of effort: the design was made with the information available at the time, and the information has changed. The report's weakness line is not a confession; it is the record of what the design learned too late to act on.

Part 2 — Common programming mistakes¶

Symptoms, and what actually causes them¶

Symptom	Cause	Fix
`collect2.exe: error: ld returned 1 exit status` with no other output	A duplicate definition (or a missing one) — this toolchain's linker names neither	Look at the symbols with `nm -C` on the objects
`multiple definition of 'loadCatalogue(...)'` on a friendlier toolchain	A non-`inline` function defined in a header included by two translation units	Move the definition to one source file, or mark it `inline`
The same for a global variable	A non-`inline` variable defined in a header	Define it once, or declare it `extern` and define it in one file
`undefined reference` to a function whose definition you can see	Its `.cpp` is not in the build command, or the definition is in an unnamed namespace	Add the file; remember internal linkage makes the definition invisible to other units
Program runs and loads nothing	The stream state was never checked after opening	Check `is_open()`, and report the failure rather than continuing
Loaded data is truncated or merged	A separator inside the data, or a hand edit	Escape the separator in the format, and validate each field on the way in
The first record is skipped when reading	A blank line or a header line treated as data	Read the header deliberately, and say so in the format documentation
Every `getline` after a `>>` returns empty	The newline left in the stream	`ignore(std::numeric_limits<std::streamsize>::max(), '\n')`, or read consistently
Memory growth before a crash during load	A count or size read from the file was trusted	Validate every value read from a file before allocating from it
A test passes after a deliberate bug is introduced	The test prints rather than asserts	Make it assert, then introduce the bug again and watch it fail
The build no longer matches the documented command	The command was written before files were added	Keep one build command in one place, and verify it from a clean state

Worked mistakes¶

M1. A definition in a header, and a linker message that names nothing¶

Two files include a header containing a function definition. The build fails with one line:

collect2.exe: error: ld returned 1 exit status

Mechanism. Each translation unit compiled successfully; each object then carried its own T shelf_capacity(). The link failed, correctly, and the toolchain chose to say so with a status code.

Diagnosis, in three commands.

g++ -std=c++17 -c a.cpp -o a.o && g++ -std=c++17 -c b.cpp -o b.o
nm -C a.o b.o | grep shelf

0000000000000000 T shelf_capacity()
0000000000000000 T shelf_capacity()

Fix. Declare in the header, define in one source file. Or mark the definition inline if it belongs to the interface and is small. Both are in examples/module-16/02-one-definition-fixed-twice/, and the second one is why that example's header contains an inline function and an inline constexpr variable.

The habit. A link error that explains nothing is answered by the symbol table, not by guessing. "What did I add recently?" is the second question; "what do the objects say?" is the first.

M2. A declaration nobody can satisfy¶

A header declares a function; its source file defines it inside an unnamed namespace:

// helper.h
namespace library { namespace internal {
int scale(int value);        // promises other files can link against this
}}

// helper.cpp
namespace {
int scale(int value) { return value * 2; }   // creates a name no other file can reach
}

helper.cpp:8:5: error: 'int {anonymous}::scale(int)' defined but not used [-Werror=unused-function]
cc1plus.exe: all warnings being treated as errors

Mechanism. Internal linkage means the definition occupies a different name from the one declared: {anonymous}::scale(int) rather than library::internal::scale(int). The declaration is unsatisfiable, the definition is unreachable, and the compiler reports the only symptom it can see — a function that is defined and never used.

Diagnosis. nm -C main.o helper.o | grep scale:

                 U library::internal::scale(int)        # wanted, and undefined
0000000000000000 t (anonymous namespace)::scale(int)    # provided, and local (lowercase t)

Fix. Decide which kind of function it is. If other files are meant to use it, define it in the same namespace the header declared, and keep the declaration. If it is this file's private helper, delete the declaration from the header and let the unnamed namespace do its job — no caller was ever supposed to see it.

M3. The open that failed, and the program that continued¶

std::ifstream in(path);
int loaded = 0;
std::string line;
while (std::getline(in, line)) { ++loaded; }
std::cout << "loaded " << loaded << " records\n";

loaded 0 records

Mechanism. The open failed; the stream is in a failed state; every getline fails immediately; the loop body never runs; the count stays zero. Every line above is individually correct, and the composition reports success.

Why it is the worst failure in this module. Nothing threw, nothing crashed, and the program then behaves exactly as if the catalogue were empty — so the next step may be to save, writing an empty file over the user's data. A crash would have been kinder.

Fix. Check the open and refuse to continue:

std::ifstream in(path);
if (!in.is_open()) {
    std::string error = "the file could not be opened";
    return false;                     // and the caller reports it, with the file name
}

The habit. For every stream: where is the first place I look at its state? If the answer is "nowhere", the code has no way to notice a missing file.

M4. `>>` then `getline`, and the field that never arrived¶

std::istringstream text("42\nCompilers|Aho|2006\n");
int year;
text >> year;
std::string record;
std::getline(text, record);

number 42, rest [] - the newline is still in the stream

Mechanism. >> stops before the delimiter and leaves it; getline reads up to the newline, and the newline is the first character it sees, so it returns an empty string. The next read then starts at the line you expected — one field later than you believed.

One-line fix. text.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); after the >>, and the same line terminates the example:

after ignore()  : number 42, rest [second line]

The habit. Choose one delimiter discipline per file reader — either getline throughout with a stringstream per line (which is what the examples do, and which keeps titles containing spaces working) or >> throughout with a deliberate ignore. Mixing them silently costs a field.

M5. The count read from the file¶

in >> count;
books_.reserve(count);          // "the file told us how many there are"

Mechanism. The file is input. A hand-edited, truncated or hostile file can say 1000000, and the program then allocates from a number it never checked — before reading a single record. This is exactly Module 4's rule about validating at the boundary, applied across a process boundary.

Fix, as the example does it:

4. count over the limit    : rejected - line 2: declared 1000000 records, over the limit of 10000
6. count that does not fit : rejected - line 2: the declared count '99999999999999999999999' does not fit in a size_t
9. a record deleted by hand: rejected - line 2: declared 3 records, found 1 - a record was added or deleted by hand

Three checks for one number, and all three matter: a limit, a conversion that can throw (std::out_of_range from std::stoul), and agreement with the data actually read. Validate before you allocate.

M6. The test that passed on a broken implementation¶

// the "test"
std::cout << schedule.pence_for(10, true) << "\n";

The implementation's cap was one day too generous. The suite printed 15, the developer read 15, and the suite passed.

Mechanism. Nothing compared the value with anything. The output was evidence for a human, and the human compared it with a number they were holding in their head — which is exactly the checking a test is supposed to do.

Fix. One assertion per claim, and the boundary cases named:

harness.check(schedule.pence_for(15, false) == 42, "15 days is still capped at 42p");
harness.check(schedule.pence_for(400, false) == 42, "a year overdue is still 42p");

How to know the fix worked: introduce the bug on purpose and watch the suite fail. In the example it fails twice, at exactly the two boundaries — which is the strongest evidence that the suite is testing the right thing and not just exercising the code.

The four questions worth memorising¶

Run these on any multi-file program, including your own capstone, before you call it finished:

Which file should define this, and is the name visible outside it? — asked of every link error, and answered by nm -C, not by guesswork.
Where do I first look at this stream's state? — asked of every file open, in either direction.
What does this do when the file is empty, truncated, or hand-edited? — asked of every reader, and answered in writing in the format documentation.
Would this test fail if the code were wrong? — asked of every assertion, and answered by introducing the bug and running it.

Continue¶

Previous: Worked examples · Next: Debugging · Module home: Module 16