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)?

8
  • 'something I read that it is considered "bad practice"' Can you clarify what is your source for this statement? Your question seems to be entirely based on this premise. Aug 18, 2021 at 16:51
  • @VincentSavard stackoverflow.com/questions/613954/… . I have read multiple blogs, SO posts and articles about checked exceptions. I seriously don't know. I used to be always in favor of checked exceptions (as shown in my example). However, maybe my question refers to Java in specific and the way checked exceptions exist in Java. Nevertheless, I know that it is a controversial topic. But, for example, in my question, what alternatives do I have? If not checked exceptions?
    – George Z.
    Aug 18, 2021 at 17:01
  • 3
    I don't see anything wrong with the implementation you provided. I'd argue there's nothing wrong with checked exceptions even if they can be misused (just like, well, everything), but making the argument in the comments under your question does not seem like the appropriate place. :) Aug 18, 2021 at 17:07
  • Does this answer your question? In Java, what are checked exceptions good for?
    – gnat
    Aug 18, 2021 at 18:29
  • Do you need the stack trace? If not, use a return value instead. Aug 19, 2021 at 5:28

2 Answers 2

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.

4
  • I would even eliminate the "or something nearby in the call stack" case. But then I'm firmly in the "checked exceptions are evil" camp. I've seen way too many case of catch(Exception e){ throw new RuntimeException(e) };
    – ptyx
    Aug 18, 2021 at 20:33
  • The immediate caller (or something nearby in the call stack) is likely to be responsible for handling the error. I am supposed to be unaware of who the caller is (when writing code), right? Also, could you expand the whether you should using exceptions to control execution flow in more detail?
    – George Z.
    Aug 19, 2021 at 11:02
  • 1
    @GeorgeZ. "I am supposed to be unaware of who the caller is (when writing code), right?" That's not a bad way to think about things but I think in practical terms, it depends on what kind of code you are writing. Not all code is meant to be called by something else. Writing code for generic reuse is a lot harder and more work than writing code for a specific one-off situation. If you write everything like it's a reusable library, you are likely over-engineering IMO.
    – JimmyJames
    Aug 19, 2021 at 14:07
  • 1
    @GeorgeZ. "Also, could you expand the whether you should using exceptions to control execution flow in more detail?" There are already plenty of questions about this here already so I don't want to go too deep here but a lot of people would argue that using try-catch for anything other than 'exceptional' error scenarios is a bad idea. It's a somewhat vague distinction and in other language communities such as Python, it's considered normal and completely OK.
    – JimmyJames
    Aug 19, 2021 at 14:14
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);
        }
   
    }
}
3
  • (It may also be reasonable for user.changePassword(newPassword) to throw an Exception to say that it won't set a weak password at that level, so that one could maybe stay...) Aug 19, 2021 at 7:27
  • I see. So, the "alternative" is to let the client with more code - do all the checking. And then I guess, the exceptions can remain as unchecked.
    – George Z.
    Aug 19, 2021 at 11:15
  • 1
    Your implementation violates the Tell, Don't Ask principle [1,2]. I believe you also have a misconception about what a responsibility is. There's nothing fundamentally wrong with OP's code (and your implementation may be acceptable in some contexts as well, even if it violates the principle). Aug 19, 2021 at 12:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.