0

Consider the following code:

public class User {
    private String password;

public void changePassword( String oldPassword,
                            String newPassword) throws WrongOldPasswordException,
                                                NewPasswordTooWeakException
{
    if (password != null) {
        if (!password.equals(oldPassword)) {
            throw new WrongOldPasswordException();
        }
    }

    if (newPassword.length() < 4)
        throw new NewPasswordTooWeakException();

    this.password = newPassword;
}

public static class WrongOldPasswordException extends Exception {

}

public static class NewPasswordTooWeakException extends Exception {

}



}

With the current API design, my presentation layer knows exactly what to do:

class UserViewController {
    User user;

void changePassword() {
    String oldPassword = view.getOldPassword();
    String newPassword = view.getNewPassword();
    try {
        user.changePassword(oldPassword, newPassword);
    } catch (WrongOldPasswordException e) {
        view.showOldPasswordDoNotMatch();
    } catch (NewPasswordTooWeakException e) {
        view.showNewPasswordIsWeak();
    }
}



}

If the changePassword method returns boolean with success/fail, my presentation layer does not know the reason. So, I either use these exceptions, or return some sort of Value Object.

If not checked exceptions (something I read that it is considered "bad practice"), then what?

If I use unchecked exceptions, then the code will be the same but the controller will have to guess for the cause of the exception (Isn't this worse)?

George Z.
  • 705

2 Answers2

2

Checked exceptions are a unique feature of Java. I'm not aware of any other major language that supports them, at least. This is a pretty good example of how many people feel about them: checked exceptions are evil. Because they don't exist in other similar languages, it's pretty easy to come to the conclusion that they aren't necessary. When working in Java, though, you need to decide whether to use them.

The main problem with checked exceptions is that they can create some challenges with interoperability. A good example is when using lambdas and stream as noted in the linked article. You can't pass, for example, Integer.parseInt() directly to map() because it throws a checked NumberFormatException. Even in more traditional Java code, it presents a challenge in that wherever you call something with a CheckedException, you must either catch it or redeclare it. In practice what happens a lot of the time is that checked exceptions are swept under the rug by wrapping them in an unchecked exception.

I'm not going to argue about whether these are a good feature or not. What I would advise, though, is that you should only use checked exceptions when:

  1. The caller should/must account for the possibility of that error occurring
  2. The immediate caller (or something nearby in the call stack) is likely to be responsible for handling the error

If these two conditions are not met, then IMO, it's almost always a bad idea to declare a checked exception on a method signature.

The scenario you describe seems to meet these criteria. A different question would be whether you should using exceptions to control execution flow like this in Java.

JimmyJames supports Canada
  • 30,578
  • 3
  • 59
  • 108
1

Throw exceptions to signal that a method can't do what it says on the tin.

For example, a method user.changePassword(String newPassword) could foresee it would have no other choice but to throw an exception if it is trying to write the new password to the database but the database is unavailable. Therefore the exception can be declared and checked.

But I think you make an all too common mistake with your User.changePassword(String oldPassword, String newPassword) method, which is that you have given it the responsibility to validate the old password, i.e. it has to do more than what it says. And you are trying to use an exception to say what it won't do, rather than what it can't do.

That doesn't mean that you rename it to user.validateAndChangePassword. If anything, validateAndChangePassword describes the function of your controller method. It will surprise nobody that a changePassword(String oldPassword, String newPassword) method on an API controller would do some validation. But at the User level, I want to just be able to change it (but let me know if the database is down).

class UserViewController {
    User user;

void changePassword() {
    String oldPassword = view.getOldPassword();
    String newPassword = view.getNewPassword();
    if (!User.isStrongPassword(newPassword)) {
        view.showNewPasswordIsWeak();
        return;
    } else if (!user.verifyPassword(oldPassword) {
        view.showOldPasswordDoNotMatch();
        return;
    }

    try {
        user.changePassword(newPassword);
    } catch (IOException e) {
        view.showError(e);
    }

}



}
Jeremy Hunt
  • 305