Should I follow the normal path or fail early?

Question

From the Code Complete book comes the following quote:

"Put the normal case after the if rather than after the else"

Which means that exceptions/deviations from the standard path should be put in the else case.

But The Pragmatic Programmer teaches us to "crash early" (p. 120).

Which rule should I follow?

Answer 1

"Crash early" is not about which line of code comes earlier textually. It tells you to detect errors in the earliest possible step of processing, so that you don't inadvertently make decisions and computations based on already faulty state.

In an if/else construct, only one of the blocks is executed, so neither can be said to constitute an "earlier" or "later" step. How to order them is therefore a question of readability, and "fail early" doesn't enter into the decision.

Answer 2

If your else statement contains only failure code then, most likely, it should not be there.

Instead of doing this:

if file.exists() :
  if validate(file) :
    # do stuff with file...
  else :
    throw foodAtMummy
else :
  throw toysOutOfPram

do this

if not file.exists() :
  throw toysOutOfPram

if not validate(file) :
  throw foodAtMummy

# do stuff with file...

You don't want to deeply nest your code simply to include error checking.

And, as everyone else has already stated, the two pieces of advice are not contradictory. One is about execution order, the other is about code order.

Answer 3

You should follow both of them.

The "Crash early"/fail early advice means that you should test your inputs for possible errors as soon as possible.
For example, if your method accepts a size or count that is supposed to be positive (> 0), then the fail early advice means that you test for that condition right at the beginning of your method rather than waiting for the algorithm to produce nonsense results.

The advice for putting the normal case first means that if you test for a condition, then the most likely path should come first. This helps in performance (as the branch prediction of the processor will be right more often) and readability, because you don't have to skip over blocks of code when trying to figure out what the function is doing in the normal case.
This advice doesn't really apply when you test for a precondition and immediately bail out (by using asserts or if (!precondition) throw constructs), because there is no error handling to skip while reading the code.

Answer 4

I think @JackAidley said the gist of it already, but let me formulate it like this:

without Exceptions (e.g. C)

In regular code flow, you have:

if (condition) {
    statement;
} else if (less_likely_condition) {
    less_likely_statement;
} else {
    least_likely_statement;
}
more_statements;

In the “error out early” case, your code suddenly reads:

/* demonstration example, do NOT code like this */
if (condition) {
    statement;
} else {
    error_handling;
    return;
}

If you spot this pattern – a return in an else (or even if) block, rework it immediately so the code in question does not have an else block:

/* only code like this at University, to please structured programming professors */
function foo {
    if (condition) {
        lots_of_statements;
    }
    return;
}

In the real world…

/* code like this instead */
if (!condition) {
    error_handling;
    return;
}
lots_of_statements;

This avoids nesting too deep and fulfils the “break out early” case (helps to keep the mind – and the code flow – clean) and doesn’t violate the “put the more likely thing into the if part” because there simply is no else part.

C and cleanup

Inspired by an answer on a similar question (which got this wrong), here's how you do cleanup with C. You can use one or two exit points there, here's one for two exit points:

struct foo *
alloc_and_init(size_t arg1, int arg2)
{
    struct foo *res;

    if (!(res = calloc(sizeof(struct foo), 1)))
        return (NULL);

    if (foo_init1(res, arg1))
        goto err;
    res.arg1_inited = true;
    if (foo_init2(&(res->blah), arg2))
        goto err;
    foo_init_complete(res);
    return (res);

 err:
    /* safe because we use calloc and false == 0 */
    if (res.arg1_inited)
        foo_dispose1(res);
    free(res);
    return (NULL);
}

You can collapse them into one exit point if there is less cleanup to do:

char *
NULL_safe_strdup(const char *arg)
{
    char *res = NULL;

    if (arg == NULL)
        goto out;

    /* imagine more lines here */
    res = strdup(arg);

 out:
    return (res);
}

This use of goto is perfectly fine, if you can deal with it; the advice to stay off using goto is directed at people who cannot yet decide by themselves whether a use is good, acceptable, bad, spaghetti code, or something else.

Exceptions

The above talks about languages without exceptions, which I highly prefer myself (I can use explicit error handling much better, and with much less surprise). To quote igli:

<igli> exceptions: a truly awful implementation of quite a nice idea.
<igli> just about the worst way you could do something like that, afaic.
<igli> it's like anti-design.
<mirabilos> that too… may I quote you on that?
<igli> sure, tho i doubt anyone will listen ;)

But here's a suggestion how you do it well in a language with exceptions, and when you want to use them well:

error return in the face of exceptions

You can replace most of the early returns with throwing an exception. However, your normal program flow, i.e. any code flow in which the program did not encounter, well, an exception… an error condition or somesuch, shall not raise any exceptions.

This means that…

# this page is only available to logged-in users
if not isLoggedIn():
    # this is Python 2.5 style; insert your favourite raise/throw here
    raise "eh?"

… is okay, but…

/* do not code like this! */
try {
    openFile(xyz, "rw");
} catch (LockedException e) {
    return "file is locked";
}
closeFile(xyz);
return "file is not locked";

… is not. Basically, an exception is not a control flow element. This also makes Operations look weird at you (“those Java™ programmers always tell us that these exceptions are normal”) and can hinder debugging (e.g. tell the IDE to just break on any exception). Exceptions often require the run-time environment to unwind the stack to produce tracebacks, etc. There are probably more reasons against it.

This boils down to: in a language supporting exceptions, use whatever matches the existing logic and style and feels natural. If writing something from scratch, get this agreed on early. If writing a library from scratch, think of your consumers. (Do not, ever, use abort() in a library either…) But whatever you do, do not, as a rule of thumb, have an exception thrown if operation continues (more or less) normally after it.

general advice wrt. Exceptions

Try to get all in-program use of Exceptions agreed on by the whole dev team first. Basically, plan them. Do not use them in abundance. Sometimes, even in C++, Java™, Python, an error return is better. Sometimes it is not; use them with thought.

Answer 5

In my opinion 'Guard Condition' is one of the best and easiest ways to make code readable. I really hate when I see if at the beginning of the method and don't see the else code because it is off the screen. I have to scroll down just to see throw new Exception.

Put the checks at the beginning so that the person reading code doesn't have to jump all over the method to read it but instead always scan it from top to bottom.

Answer 6

(@mirabilos' answer is excellent, but here's how I think about the question to reach the same conclusion:)

I'm thinking about myself (or someone else) reading the code of my function later on. When I read the first line, I can't make any assumptions about my input (except those which I won't be checking anyway). So my thought is "Ok, I know I'm going to be doing things with my arguments. but first let's 'clean them up" - i.e. kill control paths in which they're not to my liking." But at the same time, I don't see the normal case as something that's conditioned; I want to emphasize its being normal. Thus,

int foo(int* bar, int baz) {

   if (bar == NULL) /* I don't like you, leave me alone */;
   if (baz < 0) /* go away */;

   /* there, now I can do the work I came into this function to do,
      and I can safely forget about those if's above and make all 
      the assumptions I like. */

   /* etc. */
}

Answer 7

This kind of condition ordering depends on the criticallity of the code section under question and whether there are defaults that can be used.

In other words:

A. critical section and no defaults => Fail Early

B. non-critical section and defaults => Use defaults in else part

C. in-between cases => decide per case as needed