Successive success checks

Question

Most of you have probably bumped into a situation, where multiple things must be in check and in certain order before the application can proceed, for example in a very simple case of creating a listening socket (socket, bind, listen, accept etc.). There are at least two obvious ways (don't take this 100% verbatim):

if (1st_ok)
{
  if (2nd_ok)
  {
  ...

or

if (!1st_ok)
{
  return;
}

if (!2nd_ok)
{
  return;
}
...

Have you ever though of anything smarter, do you prefer one over the other of the above, or do you (if the language provides for it) use exceptions?

Answer 1

This tends to be a matter of style. Some people only like returning at the end of a procedure, others prefer to do it wherever needed.

I'm a fan of the second method, as it allows for clean and concise code as well as ease of adding documentation on what it's doing.

// Checking for llama integration
if (!1st_ok)
{
  return;
}

// Llama found, loading spitting capacity
if (!2nd_ok)
{
  return;
}

// Etc.

Answer 2

I prefer the second version.

In the normal case, all code between the checks executes sequentially, so I like to see them at the same level. Normally none of the if branches are executed, so I want them to be as unobtrusive as possible.

Answer 3

I use 2nd because I think It reads better and easier to follow the logic. Also they say exceptions should not be used for flow control, but for the exceptional and unexpected cases. Id like to see what pros say about this.

Answer 4

It depends on the language - e.g. in C++ you might well use exceptions, while in C you might use one of several strategies:

• if/else blocks
• goto (one of the few cases where a single goto label for "exception" handling might be justified
• use break within a do { ... } while (0) loop

Personally I don't like multiple return statements in a function - I prefer to have a common clean up block at the end of the function followed by a single return statement.

Answer 5

What about

if (1st_ok && 2nd_ok) { }

or if some work must be done, like in your example with sockets

if (1st_ok()  &&  2nd_ok()) { }

Answer 6

I prefer the second technique. The main problem with the first one is that it increases the nesting depth of the code, which is a significant issue when you've got a substantial number of preconditions/resource-allocs to check since the business part of the function ends up deeply buried behind a wall of conditions (and frequently loops too). In the second case, you can simplify the conceptual logic to "we've got here and everything's OK", which is much easier to work with. Keeping the normal case as straight-line as possible is just easier to grok, especially when doing maintenance coding.

Answer 7

I avoid the first solution because of nesting.

I avoid the second solution because of corporate coding rules which forbid multiple return in a function body.

Of course coding rules also forbid goto.

My workaround is to use a local variable:

bool isFailed = false; // or whatever is available for bool/true/false

if (!check1) {
    log_error();
    try_recovery_action();
    isFailed = true;
}

if (!isfailed) {
    if (!check2) {
        log_error();
        try_recovery_action();
        isFailed = true;
    }
}
...

This is not as beautiful as I would like but it is the best I've found to conform to my constraints and to write a readable code.

Answer 8

For what it is worth, here are some of my thoughts and experiences on this question.

Personally, I tend to prefer the second case you outlined. I find it easier to follow (and debug) the code. That is, as the code progresses, it becomes "more correct". In my own experience, this has seemed to be the preferred method.

I don't know how common it is in the field, but I've also seen condition testing written as ...

error = foo1 ();
if ((error == OK) && test1)) {
    error = foo2 ();
}
if ((error == OK) && (test2)) {
    error = foo3 ();
}
...
return (error);

Although readable (always a plus in my books) and avoiding deep nesting, it always struck me as using a lot of unnecessary testing to achieve those ends.

The first method, I see used less frequently than the second. Of those times, the vast majority of the time was because there was no nice way around it. For the remaining few instances, it was justified on the basis of extracting a little more performance on the success case. The argument was that the processor would predict a forward branch as not taken (corresponding to the else clause). This depended upon several factors including, the architecture, compiler, language, need, .... Obviously most projects (and most aspects of the project) did not meet those requirements.

Hope this helps.