I'm working through the first chapter of an algorithms book and the exercise looks trivial: read an integer, print its prime factorization in a readable form so that 1000 becomes 2³ * 5³.
My first attempt was 27 lines of C++. It shipped with three real bugs. The compiler was perfectly happy with all three, and none of them was a typo — each one was a short expression that reads one way to a human and another way to the compiler.
Here is what went wrong, in the order I found it.
Bug 1: aux*=i < n does not mean what it looks like
I wanted to accumulate the product of the factors found so far, to know when I had reached n:
int aux = 1;
// ...
if (aux*=i < n){
cout << i << endl;
}
if (aux*=i == n)
break;
Relational and equality operators bind tighter than compound assignment, so the compiler reads that as:
aux = aux * (i < n); // (i < n) evaluates to 0 or 1
aux gets multiplied by 0 or 1 and never becomes the product I expected. The fix is parentheses — or, better, not reaching for a compound assignment inside a condition at all:
if ((aux * i) < n) { /* ... */ }
I now add those parentheses even when I'm sure they're redundant. It costs one character and removes an entire class of bug. Rajesh Bhola's Java Operator Precedence lays out the full table; the trap is identical in C++.
Bug 2: the infinite loop I built on purpose
To "try the same divisor again" after finding a factor, I wrote this:
for (int i = 2; i < n; i++) {
if (n % i == 0) {
// ...
} else {
i--; // <- the bug
}
}
The i-- and the loop's own i++ cancel each other out. i never advances and the program spins forever. It didn't even print a useful clue — with stdout going to a pipe the buffer never got flushed, so I saw one factor and then a hang:
$ echo 12 | ./fatorando
Entre numero -> 2
$ echo $?
124 # 124 = killed by timeout
The loop header already advances i. There was never a reason to touch it inside the body. The "same divisor again" idea belongs in a while, not in the counter:
for (int i = 2; i * i <= resto; i++) {
while (resto % i == 0) {
numeros.push_back(i);
resto /= i;
}
}
That while is where factorization actually happens: divide out every copy of i before moving on. Because the smaller factors are removed first, any i that survives is prime — no primality test required.
Two bonuses came from the same change. The condition i * i <= resto can stop as soon as the remaining number is prime, taking the cost from O(n) to O(√n). And whatever is left after the loop is itself a prime factor:
if (resto > 1) {
numeros.push_back(resto);
}
97 never enters the for at all (2*2 > 97) and is printed by that if.
Bug 3: std::string indexes bytes, and superscripts are not one byte
For the pretty output I wanted exponents — 2³ instead of 2 2 2. So I mapped digits to superscript characters with a lookup string:
const string sobres = "⁰¹²³⁴⁵⁶⁷⁸⁹";
resultado += sobres[normais.find(c)]; // wrong
The output came out mangled: 12 printed as 2° * 3, and 2 to the 30th printed as 2 followed by two broken bytes. hexdump -C explained it in seconds:
"⁰¹²³⁴⁵⁶⁷⁸⁹" = e2 81 b0 | c2 b9 | c2 b2 | c2 b3 | e2 81 b4 ...
std::string::operator[] counts bytes, not characters. In UTF-8 ² is two bytes (c2 b2) while ⁰ is three (e2 81 b0), so sobres[2] did not return ² — it returned the third byte of ⁰. Indexing a multi-byte string by arithmetic is only safe for ASCII. Michael Lip's How Text Becomes Binary covers the layout of the encoding itself, and the byte/rune/grapheme confusion is language-independent — Ashwin Gopalsamy's Graphemes in Go makes the same point from another language.
The fix is an array with one element per digit, so the index is always a character index:
string paraSobrescrito(int exp) {
const char* sobres[10] = {"⁰","¹","²","³","⁴","⁵","⁶","⁷","⁸","⁹"};
string resultado;
for (char c : to_string(exp)) {
resultado += sobres[c - '0'];
}
return resultado;
}
2³⁰ now emits 32 c2 b3 e2 81 b0 — valid UTF-8. If you ever have to target a terminal that isn't UTF-8, printing 2^3 instead is a one-line change and saves the whole encoding fight. The UTF-8 / Code Page Surprise That Corrupted My Log Parser is a good reminder that the bytes are the ground truth.
Bug 4, the one that isn't a bug: counting equal factors
This part was the plan, not a mistake, but it's worth writing down because it leans on an invariant. If you push factors as you find them, they arrive in non-decreasing order — 1000 gives [2,2,2,5,5,5]. Equal values are therefore always adjacent, so counting them is one pass with two indices: no sorting, no hash map.
for (size_t i = 0; i < numeros.size(); ) {
size_t j = i;
while (j < numeros.size() && numeros[j] == numeros[i]) j++;
int expoente = j - i; // the block is [i, j)
// ... print numeros[i] with that exponent
i = j; // jump to the next block
}
Notice the for has no i++: only i = j moves the cursor. That is exactly the rule bug 2 was breaking — one place, and only one, should own a loop variable.
Result
| input | output |
|---|---|
| 12 | 2² * 3 |
| 60 | 2² * 3 * 5 |
| 1000 | 2³ * 5³ |
| 1024 | 2¹⁰ |
| 1073741824 | 2³⁰ |
| 2310 | 2 * 3 * 5 * 7 * 11 |
What I'd do differently
- Compile with
-Wall -Wextraand actually read the warnings, and put atimeoutaround anything containing awhileloop. A hang should be a one-second lesson, not a mystery. - Treat "the output is mangled" as a bytes problem before a logic problem.
hexdump -Csettled bug 3 in ten seconds. I spent much longer suspecting the algorithm. - Never mutate a loop's control variable inside the body, unless controlling that loop is the only thing the variable does.
Three unrelated bugs, one shared shape: a short expression that reads one way to a human and another way to the compiler. Reading the code out loud didn't catch any of them. Running it — and looking at the bytes — caught all three.
What's the dumbest precedence bug you've shipped? I'd like to know I'm not alone in the i-- club.
Top comments (1)
Nice write-up, and the while loop that divides out every copy of i is a clean fix. One small hardening for the loop condition: i * i <= resto can overflow a 32-bit int when the input is close to INT_MAX. A prime like 2147483647 pushes i * i past the limit at i = 46341, and signed overflow is undefined behaviour in C++. Writing it as i <= resto / i avoids the problem, and compiling with -fsanitize=undefined would have flagged it at runtime. Agreed on -Wall -Wextra as well.