Paranoid Programming: beware the "else" statement

Posted by james on Nov. 18, 2009

It's very typical to have something like this:

if (num_items == 0) {
// handle no items here
} else if (num_items == 1) {
// handle one item here
} else {
// handle more than one item here
}

The comments make this huge flaw very obvious. Without the comments, it'd be harder to spot: what if num_items is -1? This falls through the cracks, since we use the else as a "catch all" bucket, them assume that it's valid data. Unless we've validated the data before, this is a bug. But even if you remember most of the time to validate data, one time you forget and you've introduced a bug. So you're simply shifting the bug from one place to another.

Another variation of this:

if (num_items == 0) {
// handle no items here
} else if (num_items == 1) {
// handle one item here
} else if (num_items > 1) {
// handle more than one item here
}

This code is "more correct", but still has the same bug; if num_items is -1, then your code wont execute in any of the if() blocks. This may be intentional, but most often it's not.

So to prevent these common and subtle bugs, always use the else as a catch-all error.

if (num_items == 0) {
// handle no items here
} else if (num_items == 1) {
// handle one item here
} else if (num_items > 1) {
// handle more than one item here
} else {
// Shouldn't get here, bad input
assert(false);
}


It's so easy to add the "else { assert(false); }", and it makes your code so much safer. Validating user input doesn't replace this. If you refactor an internal function and it starts calling this function with bad data, or if you make a mistake further up in the same function, you're still sunk unless you're being paranoid.

Moral: always be explicit with if/else conditions, don't rely on else to catch the "probably is good data" unless you're sure, and use else/asserts to let you know when data isn't what you expected.