Why Return Codes Make Me Mad


In which I go on and on complaining about return codes, letting off some steam.

So, return codes. They make me mad. When given the choice, I always prefer exceptions.

Let’s say you have a function getValue(), which returns, well, the value of something. However, if something went wrong during setup, the value might never have been set, and we need to detect that. What we would like to write is

type value = getValue(); //For some type bool / int / WhateverClass

We do however need to detect error conditions. Either we use exceptions, or we use return codes. We can do either

try {
    type value = getValue();
} catch (NotInitializedException&) {
    //handle
}

or

type value;
bool ok = getValue(value)
if (!ok) {
    //handle
}

Either way you need to look out for errors, and maybe one isn’t so much worse than the other. What return codes do to the flow of you code however, is much worse. Here are some of the things that annoy me the most:

The size of the code explodes
An innocent, readable line like

if (getValue() == whatever)

turns into this mess:

type value;
getValue(value)
if(value == whatever)

Which one is easiest to read?

And of course, you can’t do this anymore:

finalResult = doStuffWith(getValue());

Variables have intermediate, invalid values
In the previous example, value needs to be constructed before getValue() is called. There’s a whole list of problems with this:

  • If it is a fundamental type (int, float, bool etc.) you have to manually initialize it, or you will have a dangerous uninitialized variable.
  • Some types don’t even have a default constructor
  • Some types are expensive to construct
  • Some constructors have side effects
  • value will have a meaningless value for a (short) part of the life of the program

You cannot use const variables.
I always make variables const whenever I can. This helps with program correctness, because I will never accidentally modify them, or pass them to a function that modifies them. But now you can’t do

const type value = getValue();

you have to do

type value; //Can't use const
getValue(value) //because it's modified here

Error checking code will infect your code and make it unreadable

Instead of handling errors in one place like this:

try {
    const type value = getValue()
    const othertype othervalue = getRelatedValue(value);
    doStuff(othervalue);
} catch (NotInitializedException&) {
    //handle;
}

You have to check for errors all the time:

type value;
bool ok = getValue(value);
if (ok) {
    othertype othervalue;
    bool otherok = getRelatedValue(othervalue, value);
    if (otherok) {
        doStuff(othervalue);
    } else {
        //handle
    }
} else {
    //handle
}

Look at this mess! Notice how the error handling code gets intermixed with the happy code. In the version of the code using exceptions, this is not necessary, as the rest of the try block is automatically skipped when an exception is thrown.

Constructors can’t even return a return code
The return value of a constructor always is the constructed object, so here you don’t even have a choice. If you don’t want to use exceptions, you need to remember that the object is now in an invalid state. So either you hope your users remember to do

SillyType s;
if (!s.gotProperlyConstructed() {
    //handle
}

or you need to make the newly constructed object remember that it is invalid, and return that as an error code indicating that every time someone calls a method on it from now on.

The code does not say what it really means
An argument to a function is an argument to a function. The return value of a function is the return value of a function. When reading a piece of code, this is the natural way to reason about it. When arguments are suddenly used for return values, the natural flow of the code is broken.

That’s not all
In addition, there’s a host of other problems like:

  • It’s easy to forget to check a return code, leaving your program silently in an invalid state. However, if you forget to catch an exception, you program blows up and at least you know something is wrong.
  • Operators can’t use return codes.
  • Exceptions tend to have useful information attached to them, describing the error. Return codes are usually enums, integers or booleans. (This can be fixed though, by returning an error object instead of an error code.)

Readable code
In all of the examples above, return codes lead to code that is less readable. And for me, readable code is extremely important. When your code is unreadable:

  • It is easier for bugs to hide
  • It is harder to make changes
  • It takes longer to get to know the code for new programmers (which might be yourself, half a year later)
  • It is usually harder to write tests
  • It is scary to refactor because you don’t understand the code fully, so you end up patching on another few lines and another few lines instead

In short, unreadable code leads to more bugs, longer development time for new features, more time spent in the debugger, and more time spent on maintenance. This is why I hate unreadable code. This is why return codes make me mad.

What can you do?
For new projects, go with exceptions. For legacy code you usually need to stay with the existing style. If all the code around you uses return codes, you probably should too. I would however argue that there still can be a place for exceptions: If you are extending the code base with an isolated module, or doing a major refactoring of one, and the interface to the rest of the code is small, use exceptions internally. Then catch the exceptions on the boundary and translate to return codes. After a while, you will gradually move towards exceptions and more readable code for the entire code base.

If you enjoyed this post, you can subscribe to my blog, or follow me on Twitter.

Disempower Every Variable


In which I argue you should reduce the circle of influence, and the ability to change, of every variable.

The more a variable can do, the harder it is to reason about. If you want to change a single line of code involving the variable, you need to understand all its other uses. To make your code more readable and maintainable, you should disempower all your variables as much as possible.

Here are two things you can do to minimize the power of a variable:

1: Reduce its circle of influence (minimize the scope)
I once had to make a bugfix in a 400 line function, containing tens of for-loops. They all reused a single counter variable:

{
  int i;
  (...)
  for (i = 0; i < n; ++i) {
  }
  (...)
  for (i = 0; i < n; ++i) {
  }
  //350 lines later...
  for (i = 0; i < n; ++i) {
  }
}

When looking at a single for-loop, how am I to know that the value of i is not used after the specific loop I was working on? Someone might be doing something like

for (i = 0; i < n; ++i) {
}
some_array[i] = 23

or

for (i = 0; i < n; ++i) {
}
for (; i < m; ++i) {
}

The solution here is of course to use a local variable to each for-loop (unless of course it actually is used outside of the loop):

for (int i = 0; i < n; ++i) {
}
for (int i = 0; i < n; ++i) {
}

Now I can be sure that if I change i in one for-loop, it won’t affect the rest of the function.

2: Take away its ability to change (make it const)

(I have blogged about const a few times before. It is almost always a good idea to make everything that doesn’t need to change const.)

Making a local variable const helps the reader to reason about the variable, since he will instantly know that its value will never change:

void foo() {
  const string key = getCurrentKey();
  (...) //Later...
  doSomethingWith(key);
  (...) //Even later...
  collection.getItem(key).process();

Here the reader knows that we are always working with the same key throughout foo().

In summary: Reduce the circle of influence (by reducing the scope) and take away the ability to change (by using const).

If you enjoyed this post, you can subscribe to my blog, or follow me on Twitter.