Java coding standard: document when to use/not use checked exceptions

[pyokagan]

Unchecked exceptions are used for:

• Errors which are unrecoverable or very hard to recover from (e.g. out of memory)
• Programmer errors (e.g. division by zero, null pointer exceptions)

Checked exceptions are for errors which are (1) recoverable (2) even after doing everything in our power to predict/prevent the possible errors, still cannot be predicted without actually running the operation because the resource the operation in operating on is not within the program's control.

As the SO answer notes, the Java stdlib is not very consistent in its use of checked/unchecked exceptions either, which contributes the confusion.

Example 1: Reading from a file

We will be temped to do the following when attempting to read from a file:

if (fileExists(filename)) {
    readFromFile(filename);
} else {
    print(filename + " does not exist");
}

However, this is not correct. The file might be deleted in between the "fileExists" and "readFromFile" calls by another process.

In this case, we won't truly know if the file exists unless we actually read from the file. Thus, it is proper for readForFile to throw a checked exception, i.e.

try {
    readFromFile(filename);
} catch (FileNotExistsException e) {
    print(filename + "does not exist");
}

Example 2: Maintaining invariants on a list

The addressbook-level4 has lots of instances of:

try {
    uniqueList.add(item);
} catch (DuplicateItemException e) {
    print("Item is a duplicate");
}

Following the rules set forth above, we can see that the DuplicateItemException is completely avoidable since the program is completely in control of the uniqueList, and thus can properly ensure that all preconditions are met. The proper implementation should be:

if (!uniqueList.contains(item)) {
    uniqueList.add(item);
} else {
    print("Item is a duplicate");
}

References:

• https://stackoverflow.com/a/19061110

  Checked Exceptions should be used for expected, but unpreventable errors that are reasonable to recover from. Unchecked Exceptions should be used for everything else.

• https://docs.oracle.com/javase/tutorial/essential/exceptions/runtime.html

  Here's the bottom line guideline: If a client can reasonably be expected to recover from an exception, make it a checked exception. If a client cannot do anything to recover from the exception, make it an unchecked exception

[pyokagan]

even after doing everything in our power to predict/prevent the possible errors, still cannot be predicted without actually running the operation.

Okay, maybe this could be better explained as: "accessing a resource which the program has no control over" 🤔

[pyokagan]

@damithc @Zhiyuan-Amos @yamgent Thoughts? There really needs to be a consistent approach inside addressbook's code base.

[Zhiyuan-Amos]

Following the rules set forth above, we can see that the DuplicateItemException is completely avoidable since the program is completely in control of the uniqueList, and thus can properly ensure that all preconditions are met.

Yup I agree with the this. As such, I suppose Model's methods should return true / false instead of throwing Exceptions (for e.g. when adding a duplicate person into the UniquePersonList, then we return false to show that the adding failed). Then for the Commands, should we still throw CommandException when we the the Model's methods return false?

[pyokagan]

@Zhiyuan-Amos

I suppose Model's methods should return true / false instead of throwing Exceptions (for e.g. when adding a duplicate person into the UniquePersonList, then we return false to show that the adding failed)

Won't that hide programmer errors? e.g. if the return value is accidentally not checked.

[yamgent]

if (!uniqueList.contains(item)) {
    uniqueList.add(item);
} else {
    print("Item is a duplicate");
}

1. uniqueList.add() will still throw DuplicateItemException, but it is an unchecked exception, because it qualifies as a programmer error. Am I right?

2. The decision to use checked or unchecked exceptions is made in the API level. Suppose:

   • I have an API interface UniqueList.
   • I have two different implementation of unique list, class UniqueNonConcurrentList implements UniqueList, and class UniqueConcurrentList implements UniqueList.
   • UniqueConcurrentList violates the rule that errors pertaining to uniqueList.add() is always avoidable/preventable, while UniqueNonConcurrentList does not violate it.
     
     Should interface method uniqueList#add() use a checked or unchecked exception?

[pyokagan]

@yamgent

uniqueList.add() will still throw DuplicateItemException, but it is an unchecked exception, because it qualifies as a programmer error. Am I right?

Yes. And right, this will need to be elaborated on. I think I'll just open a PR soonish.

The decision to use checked or unchecked exceptions is made in the API level.

Right, a fully specified API definition will define the preconditions/postconditions of the method, what kind of exceptions are thrown etc.

Should interface method uniqueList#add() use a checked or unchecked exception?

I think it should use a checked exception. Otherwise, UniqueConcurrentList will violate LSP.

[damithc]

@damithc @Zhiyuan-Amos @yamgent Thoughts? There really needs to be a consistent approach inside addressbook's code base.

I support the proposal as it sounds right in principle. Let's give it a try and see if holds up well in practice too.

[pyokagan]

To add: checked exceptions, when thrown, should leave the program in a documented state. This state needs to be documented in the javadoc of all methods which has the "throws" declaration. This is needed to guide callers on recovering from the exception,

It is assumed that unchecked exceptions, when thrown, leave the program in an undefined state, and thus not worth the effort to recover from.