Java flow control by throwing exceptions

Question

I have a question that is bothering me for a while about the preferable flow control method.

I often encounter a situation, where I have to decide what to do based on return value from a method which is null or not null

So I have two options which I know about, how to deal with it:

Check for null:

public class BasedOnNullFlowControl {

    public String process(String login) {
        String redirectUri = getRedirectUri(login);
        if (redirectUri != null) {
            return redirectUri;
        } else {
            return "Your login is taken";
        }
    }

    private String getRedirectUri(String login) {
        Optional<Login> loginFromDb = checkLoginExists(login);
        if (loginFromDb.isPresent()) {
            return null;
        } else {
            return "some-redirect-url";
        }
    }

    private Optional<Login> checkLoginExists(String login) {
        return Optional.empty(); //assume this value comes from some API
    }

    private class Login {}
}

Or interrupt the flow with exception:

public class ExceptionFlowControl {

    public String process(String login) {
        return getRedirectUri(login);
    }

    private String getRedirectUri(String login) {
        Optional<Login> loginFromDb = checkLoginExists(login);

        loginFromDb.ifPresent(l -> {
            throw new LoginExistsException();
        });
        return "some-redirect-url";
    }

    private Optional<Login> checkLoginExists(String login) {
        return Optional.empty();
    }

    private class LoginExistsException extends RuntimeException {
    }

    private class Login {}
}

I know, that exceptions should be used only in exceptional situations, but my situation is not exceptional and still second solution looks better for me, because I can add some exception handler (like in Spring) and translate it to some nice Http status code in the end. The controller method process is not contaminated with dozens null checks.

Could you smart people please advise which one solution should be used? Or maybe there is a third one?

Answer 1

First of all, if you use optionals you can avoid the null checks altogether:

public class BasedOnNullFlowControl {

    public String process(String login) {
        String redirectUri = 
        return getRedirectUri(login).orElse("Your login is taken");
    }

    private Optional<String> getRedirectUri(String login) {
        return checkLoginExists(login).map(ifExists -> "some-redirect-url");
    }

    private Optional<Login> checkLoginExists(String login) {
        return Optional.empty(); //assume this value comes from some API
    }

    private class Login {}
}

Next, your second version only looks nice with unchecked exceptions. So this boils down to checked vs. unchecked exceptions. Which is a kind of religious question.

A few years ago I've worked with the software where the previous "generation" of developers followed the unchecked exceptions religion. Like, "we mostly can't handle exceptions reasonably anyway so let's just throw unchecked exceptions and have some routine on the top level to catch them and show the error info dialog to the user". That failed spectacular and it was extremely difficult to refactor it. The code was like a mine field, you could never know what could or could not happen.

After that I've completely switched to the "checked exceptions religion". If you have a situation which would be better described as a programming error (like, passed null where no null is expected or access to classpath resource which MUST be there), then a runtime error is suitable. In other cases model and use checked exceptions.

Answer 2

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";