In addition to fulfilling its primary responsibility, a method often has to handle potential error conditions. You can organize the control flow of such a method in many ways. I once had a manager who told me he preferred to see the path with no errors first, followed by error handling code. This was back when I was programming in C, but the style I adopted then still echoes in my code today. For example, I might structure a method that handles an HTTP POST from a form like this:

if (userIsSignedIn) {
    // The user is indeed signed in, so keep going...
    if (finished) {
        // They pressed finished, so keep going...
        if (!formErrors) {
            // Actually do what the user was trying to
            // accomplish with this form.
        }
        else {
            // handle form errors
        }
    }
    else {
        // they pressed cancel, so abort
    }
}
else {
    // handle user not signed in problem
}

My C code back in those days, therefore, often had the shape of a big right angle bracket, with several cascading ifs going in, then the "success" code, followed by elses handling all the potential errors the method might encounter.

When I was discussing this issue with a fellow programmer (not a manager) back in those days, he said that he prefered a method's code to resemble more of a pipe character (|) than an angle bracket (>). He wanted the code to be flat against the left hand side. Thus he'd write the above code like this:

if (!userSignedIn) {
    // handle user not signed in problem
    return;
}

if (!finished) {
    // they pressed cancel, so abort
    return;
}

if (formErrors) {
    // handle form errors
    return;
}

// Actually do what the user was trying to
// accomplish with this form.

The benefit with this approach is that my colleague avoided the cascading indentation, but at the cost of multiple returns. I had heard some programmers complain that having multiple returns inside a method makes it harder to understand. I myself didn't find such methods harder to understand, but now that I've started using Scala for a few things, I've begun to see methods more as expressions. This mindset implies that a method contains just one expression that resolves to a value, which is returned.

But even if you take the single return approach in a method, you still may prefer to handle the errors first, because they are often shorter than the "success" code. That might look like this:

if (!userIsSignedIn) {
    // handle user not signed in problem
}
else if (!finished) {
    // they pressed cancel, so abort
}
else if (formErrors) {
    // handle form errors
}
else {
    // Actually do what the user was trying to
    // accomplish with this form.
}

The benefit with this approach is that you can avoid both multiple returns from the method and the cascading indentation, but the errors are on top. Some may actually find this easier to read, but my old manager at least wouldn't like having to scan all the way to the bottom to find the code that we hope will be executing most of the time.

Lastly, one other possibility is to use exceptions to structure the flow of the method. For example:

try {
    User user = getSignedInUser();

    ensureFinishButtonPressed();

    Map formFields = getFormFields();

    // Actually do what the user was trying to
    // accomplish with this form.
}
catch (UserNotSignedInException unsie) {
    // handle user not signed in problem
}
catch (UserPressedCancelException upce) {
    // they pressed cancel, so abort
}
catch (FormErrorsException fee) {
    // handle form errors
}

private User getSignedInUser() {
    if (!userIsSignedIn) {
        throw new UserNotSignedInException();
    }
    // else return the signed in User
}

private void ensureFisishButtonPressed() {
    if (!finished) {
        throw new UserPressedCancelException();
    }
}

private Map getFormFields() {

    if (formErrors) {
        throw new FormErrorsException();
    }
    // populate a map with form fields and return
}

The previous example is very contrived, but should demonstrate the potential structure. When I first read about exceptions in Java, I noticed that it allowed me to put error handling code after the success code. It flattened the success code and made it easier to read. But I didn't adopt exceptions for this kind of situation, in which I'm really trying to implement one method. (Notice that the methods throwing exceptions are all private, created solely to help me implement the HTTP POST handler method.)

My question today has two parts. First, which of these approaches (or perhaps you have a different approach to suggest) would you recommend for structuring the flow of methods that must handle potential errors? Second, in my experience, each individual programmer on a project would use his or her own preferred style of structuring such flow—in other words, how flow was structured was decided by personal coding style. Most people who participated in the discussion of my previous blog post seemed to feel that establishing a code style for a project was a good idea. But how far should that style policy go? Should it only deal with formatting issues such as curly brace placement? Or should it actually address coding structure issues, such as the one presented in this blog post? Please post your opinions in the discussion forum.

    User user = getSignedInUser();  // could throw exception
    if (finished) {
        Map<String, String> fields = getFormFields(); // could throw exception
        // ...process the form....
    } else {
        // ...abort handling the form
    }

interface Handler {
  void handle(Request, Response) throws ???Exception(s?);
}
 
LoginHandler implements Handler {
  public void handle(Request, Response) throws ... {
    // notice how this has the if/else problem as well, just smaller
    if (request.user.loggedIn) request.add(request.user.asLoggedIn());
    else throw ...
 
    _decorated.handle(request, response);
  }
}
 
FormHandler implements Handler {
  public void handle(Request, Response) throws ... {
    if (valid(request.data)) request.add(request.data.asBusinessObjects());
    else throw ...
 
    _decorated.handle(request, response);
  }
}
 
BusinessHandler implements Handler {
  public void handle(Request, Response) throws ... {
    assert request.user.loggedIn;
    assert FormHandler.valid(request.data); // yuck!
 
    // do work
  }
}

if (!userSignedIn()) {
    // handle user not signed in problem
    return;
}

do_something();

if (!finished()) {
    // they pressed cancel, so abort
    return;
}

do_something_else();

if (formErrors()) {
    // handle form errors
    return;
}

// Actually do what the user was trying to
// accomplish with this form.

private void process_form() {
    if (!formErrors) {
        // Actually do what the user was trying to
        // accomplish with this form.
    }
    else {
        // handle form errors
    }
}

[...]

boolean ok = false;
if (!userIsSignedIn) {
    // handle user not signed in problem
}
else {
    if (!finished) {
        // they pressed cancel, so abort
    }
    else {
        ok = true;
    }
}
if (ok) process_form();

if (!file.exists()) {
    print("Error: file does not exist");
    return;
}
text = file.read();  // "in-between" statement
if (text == null) {
    print("Error: failed to read file");
    return;
}
lines = text.split();  // "in-between" statement
if (lines.size() == 0) {
    print("Error: no header found");
    return;
}
return parser.parse(lines);