Java: Exceptions as Control Flow

Java: Exceptions as control flow?

Generally exceptions are expensive operations and as the name would suggest, exceptional conditions. So using them in the context of controlling the flow of your application is indeed considered bad practice.

Specifically in the example you provided, you would need to do some basic validation of the inputs you are providing to the StringMatch constructor. If it were a method that returns an error code in case some basic parameter validation fails you could avoid checking beforehand, but this is not the case.

Avoid using exceptions as flow control

It's a design smell, the pattern that was recognized by sonar matches your code, which is most likely the case if you are expecting something to happen that will happen in usual flow, and then throw an exception instead of handling the error and providing a proper return instead of throwing another exception to the caller.

One reason this is not optimal is that exception handling is signifcantly slower than simply returning null (for example) in an expected error case. Also your software should provide a clear API which should not be Exception centric.

Java flow control by throwing exceptions

The exception way allows for less dependency between the parts of the code, so the first way is quite poor. With it you need to pass a null value loaded with special meaning (null == login taken) all the way from checkLoginExists() to process.

However It's not the job of getRedirectUri() to throw the exception, it should be done in checkLoginExists(). I'm a dubious about having getRedirectUri() be responsible for checking whether a login exists or not too. Not to mention that using null or Optional only gives you a binary notion of success/fail. What if the login exists, but is locked, so you have to forbid login as well as creating a new user with the same name. You might want to redirect to a different page to indicate that the login is locked.

Still, this is opinion based, highly dependent on the specifics of the situation and there isn't a clear cut rule that you could follow.

Edit (now that all this hubbub is over with): It is a widely accepted opinion that normal flow control should not be done with exceptions. However the definition of normal is not so easily defined. You wouldn't write code like

int[] array = ...
int i = 0;
try {
    while(true) {
       array[i]++;
       i++;
    }
} catch(ArrayIndexOutOfBoundsException e) {
    // Loop finished
}

However if you're not using a ridiculously simple example like above, it goes into the gray area.

Note also that exception is not the same as an application error. It is impossible (or at least not sensible) to try to work around exceptions by validating everything. If we consider the presented case where a new user is trying to register a username, and we want to prevent duplicate usernames. You can check for a duplicate and show an error message, but there's still the possibility that two concurrent requests attempt to register the same username. At this point one of them will get an exception, as the database will disallow duplicates (if your system is sensible in anyway).

Validation is important, but there's nothing truly exceptional about exceptions. You need to handle them gracefully, so why bother validating if you end up with having to handle an exception anyway. If we take an example case of something I wrote in the comments, where you need to prevent duplicates and curse words as usernames, you might come up with something like this (let's assume this is in getRedirectUri():

if(checkForCurseWords(username))
    return "login not allowed";

try {  // We can also omit the try-catch and let the exception bubble upwards
    createLogin(username);
} catch(DuplicateException e) {
    return "login exists";
} catch(Exception e) {
    return "problem with login";
}
return "main page";

Using exceptions for control flow

Using exceptions works like anything else you do. You can use it but don't overdo it.

In general you want to throw an exception when something happened that was not right, but your program can recover from, probably in another module. Exceptions help to put the program (more precisely the run-time stack) in a good state, so you can do something about the error.

Using return values is usually not a good idea and often seen as not-so-good design.

In your case the exception will most likely trigger a message to the user, which happens in the UI and should be apart from the signup logic itself, so using an exception seems appropriate.

Now for the part of overdoing it. You can easily do a single exception such as SignupException that encloses a reason why it went wrong. You probably don't want to end up with a system that has more exception classes than classes with productive code.

Is using exceptions for normal control flow a practice to be discouraged or not?

Joshua Bloch summarizes in "Effective Java":

exceptions are, as their name implies, to be used only for exceptional conditions; they should never be used for ordinary control flow

You can find the corresponding snippet of the book ("Item 57: Use exceptions only for exceptional conditions") on Google books.

Besides the reason that exception-abuse can lead to code that's hard to understand Bloch has some reason that is valid for today's JVM implementations:

Because exceptions are designed for exceptional circumstances, there is little
incentive for JVM implementors to make them as fast as explicit tests.

In case of the (Hotspot) JVM it's pretty expensive to create an exception (think of the generated stacktrace).

Throwing exceptions to control flow - code smell?

It entirely depends on what that error condition is, and what the method's job is. If returning ERROR is a valid way of handling that error for the calling function, why would it be bad?

Often, however, it is a smell. Consider this:

bool isDouble(string someString) {
    try {
        double d = Convert.ParseInt32(someString);
    } catch(FormatException e) {
        return false;
    }
    return true;
}

That is a very big code smell, because you don't expect a double value. You just want to know whether a string contains a double.

Sometimes, the framework you use doesn't have other ways of doing what you want. For the above, there is a better way:

bool isDouble(string someString) {
    bool success;
    Convert.TryParseInt32(someString, ref success);
    return success;
}

Those kinds of exceptions have a special name, coined by someone whose blog I read recently, but sadly, I forgot its name. Please comment if you know it. Last but not least, the above is pseudocode. I'm not a C# developer so the above doesn't compile, I'm sure, but TryParseInt32 / ParseInt32 demonstrates that well I think, so I'm going with C#.

Now, to your code. Let's inspect two functions. One smells, and the other doesn't:

1. Smell

public int setupSystem() {
    doA();

    try { doB(); }
    catch (MyException e)
    { return ERROR; } 

    doC();
    return SUCCESS;
}

That's a code smell, because when you want to setup a system, you don't want it to fail. Failing to setup a system means you can't continue without handling that error.

2. OK

public int pingWorkstation() {
    doA();

    try { doB(); }
    catch (MyException e)
    { return ERROR; } 

    doC();
    return SUCCESS;
}

That is OK, because the purpose of that method is to test whether the workstation is still reachable. If it's not, then that is part of the result of that method, and not an exceptional case that needs an alternative return path.

---

Related Topics