In the last post of this little series I focused on unit testing. In later posts I will deal with further topics of error handling, code commenting, the length of the methods, and readability issues.
Checked versus unchecked exceptions
You may remember, that during a job interview the chief architect of the company that issued the little programming exercise to me, discussed various aspects of that application with me. Regarding error handling, he specifically called my attention to the following items:- The preference of checked exceptions over unchecked exceptions.
- The light-hearted use of exceptions.
- The throwing of an exception by a constructor.
- The apparent swallowing of an exception by the part of the application that catches it.
The short chapter seven of «Clean Code» is dedicated to error handling. The one point it stresses with vigor is: Do not use checked exceptions!
The price of checked exceptions is an Open/Closed Principle violation. If you throw a checked exception from a method in your code and the catch is three levels above, you must declare that exception in the signature of each method between you and the catch. This means that a change at a low level of the software can force signature changes on many higher levels.The open/closed principle states that software modules should be open to extension and closed to modification.
—Robert C. Martin, «Clean Code», page 107
When a single change to a program results in a cascade of changes to dependent modules, that program exhibits the undesirable attributes that we have come to associate with “bad” design. […] The open-closed principle attacks this in a very straightforward way. It says that you should design modules that never change.
—Robert C. Martin, «The Open-Closed Principle», 1996
Let's accept this for now. My little bowling applications has only one spot where checked exceptions originate from: the method «addRoll(int)». It is the focal point of the whole application. All the more complicated computations happen here. All other methods either serve it or feast on it. Its skeleton looks like this:
public void addRoll(int roll) throws BowlingException {
if (isComplete())
throw new BowlingException("No more rolls allowed.");
if (roll < 0 || roll > 10)
throw new BowlingException("Roll out of bounds.");
if (mExtraRolls > 0) {
…
} else if (mFrameState[mCurrentFrame] != FrameState.FIRST_ROLLED) {
…
} else if (roll + mRolls[mCurrentRoll - 1] <= 10) {
…
} else
throw new BowlingException("Frame out of bounds.");
}
It throws a BowlingException in three spots, and although one is thrown at the very end of the method—at least in terms of writing—they all signal a violations of a precondition: Either the game is already finished, i.e. all frames have been rolled, or an individual roll was out of range (either a negative number or above ten), or the sum of two rolls of the same frame exceeded ten in sum.Violations of precondition make good candidates for unchecked exceptions, remarks Joshua Bloch in «Effective Java» (page 244), because they often indicate programming errors. Programming errors are often not recoverable. For Mr. Bloch, this is the main criterion for choosing between checked and unchecked exceptions:
Use checked exceptions for conditions from which the caller can reasonably be expected to recover. […] By confronting the API user with a checked exception, the API designer presents a mandate to recover from the condition.You see, he is far less rigorous about the use of checked exceptions than Uncle Bob. Joshua Bloch does not mention the breach of encapsulation as an argument against checked exceptions. He brings, however, an another argument for favoring unchecked exceptions over checked exceptions that «Clean Code» misses: In places where the programmer knows that he will not violate the preconditions, he is not forced to injure the readability of his code my polluting it with try-catch-clauses that will never trigger!
—Joshua Bloch, «Effective Java», page 244
In the bowling example this would mean: if the «user interface» (i.e. main()) would check the validity of the numbers entered itself, and not merely pass them whatever-they-may-be to the BowlingGame class, then the whole try-catch-construct would be unnecessary and could disappear—if an unchecked BowlingException was thrown by «addRoll(int)»!
Unfortunately, the two heuristics «checked exceptions, if the error can and should be recovered from» and «unchecked exceptions for programming errors, like most precondition violations are» are competing in case of the bowling example. The BowlingException of «addRoll(int)» certainly indicates a violation of a precondition. But it also signals an issue that the application can and should deal with! The real problem here is not for or against checked exceptions!
Imagine we would decide to not use exceptions at all and use a different way of signaling an error condition. There are quite a few option how to do this. Returning an error code is the most common. Changing internal state that can be queried by the client is another, and using a call-back-method yet another. Would our problem change if we decided for one of these option instead of throwing an exception? I would not! The true problem is that the «user interface», i.e. the main()-method, depends on the error code for doing its job! There is no other way for it to check that a roll entered by the user is not acceptable in the running game! The application lacks an API that prevents the occurrence of errors!
One technique for turning a checked exception into an unchecked exception is to break the method that throws the exception into two methods, the first of which returns a boolean that indicates whether the exception would be thrown.Let's see what that means for the bowling application. The main()-method that contains all the code responsible for dealing with user input, currently looks like this—never mind the German, I will fix that too:
—Joshua Bloch, Effective Java, page 247
public static void main(String[] args) {
BowlingGame game = new BowlingGame();
System.out.println("BOWLING! Bitte Ergebnisse eingeben!");
int roll = 0;
do {
int curFrame = game.getCurrentFrame();
int curRoll = game.getCurrentRoll();
System.out.printf("Ergebnis für Frame %d, Wurf %d: ", curFrame,
curRoll);
try {
roll = new Scanner(System.in).nextInt();
} catch (InputMismatchException ex) {
System.out
.println("Bitte Ganzzahl zwischen 0 und 10 eingeben!");
continue;
}
try {
game.addRoll(roll);
} catch (BowlingException ex) {
ex.printStackTrace();
System.err.println(ex.getMessage());
}
if (game.isSpare(curFrame))
System.out.print("SPARE! ");
else if (game.isStrike(curFrame))
System.out.print("STRIKE! ");
System.out.printf("Zwischenstand: %d%n", game.getScore());
} while (!game.isComplete());
System.out.printf("Endstand: %d%n", game.getScore());
}
We are after the second try-catch-clause, but notice that the first one follows the same pattern and could be fixed in the same manner by calling «hasNextInt()». After re-factoring, main() should look like this:public static void main(String[] args) {
BowlingGame game = new BowlingGame();
do {
…
if ( game.canAddRoll(roll) ) {
game.addRoll(roll);
}else {
System.out.println("This is not a valid roll!");
}
…
} while (!game.isComplete());
System.out.printf("Endstand: %d%n", game.getScore());
}
First, we turn BowlingException into an unchecked exception by inheriting from RuntimeException. Many signatures of the test-suite change due to that, as they no longer must declare to throw a BowlingException. Then we add unit tests for canAddRoll(int). Many of the tests mirror those for checking the throwing of BowlingException by addRoll(int). By now, there are so many test-cases pertaining to error handling that moving them into a separate class «ErrorHandlingTests» seems admissible. Finally, canAddRoll(int) can be implemented:
public boolean canAddRoll(int roll) {
return !(isComplete()
|| (roll < 0 || roll > 10)
|| (!isFirstRoll() && mExtraRolls==0 &&
(roll + mRolls[mCurrentRoll - 1] > 10)));
}
These are the precondition checks which you saw in addRoll(int) above. Their original occurrences can now be replaced by a single call to our new check function:
public void addRoll(int roll) {
if (!canAddRoll(roll))
throw new BowlingException(roll, mRolls);
if (mExtraRolls > 0) {
…
} else if (mFrameState[mCurrentFrame] != FrameState.FIRST_ROLLED) {
…
} else {
…
}
}
Two things are noteworthy about the implementation of canAddRoll(int). First, some of its checks are still duplicated by addRoll(int), but there is little we can do about it. Second, the meaning of the boolean expression is not easy to grasp. Still, a small step towards better readability has been taken: The method «isFirstRoll()» is new. In essence it corresponds to the (negated) expression in line 6 of the previous listing. It was just too awkward to have that beast also in the already complicated boolean expression that defines canAddRoll(int). But since readability issues are not the main concern of this post, I will make no further moves in that direction.
I introduced two more subtle changes that regard error handling. The first is a more explicit documentation of the preconditions whose violation would cause a BowlingException to be thrown:
/** …
*
* @throws BowlingException
* if the roll is not within range 0–10, if the current frame
* would sum up to more than 10, or if the game is already
* complete, i.e. 10 frames have been rolled.
*/
public void addRoll(int roll) {
…
}
Also note, that the thrown exception is documented although it is unchecked and no longer appears as part of the signature of addRoll(int). Both (non-)changes realize item 62 of Joshua Bloch's «Effective Java»: «Document all exceptions thrown by each method», because
A well-documented list of the unchecked exceptions that a method can throw effectively describes the preconditions for its successful execution.The second subtle change is the introduction of a parameterized BowlingException:
—Joshua Bloch, «Effective Java», page 252
public class BowlingException extends RuntimeException {
private static final String MESSAGE
= "Cannot add roll %d to current game with rolls %s";
public BowlingException(int roll, int[] currentGameRolls) {
super(String.format(MESSAGE, roll,
Arrays.toString(currentGameRolls)));
}
}
Both Martin (page 107) and Bloch (item 63) stress the importance of informative error messages. It helps to locate the source of an error faster. The idiom for letting the exception take care of the formatting of the error message is taken from Bloch. It further reduces clutter in the business code. The exception could also store the parameters for later evaluation by the error handler. It is mandatory in cases where the client could otherwise be tempted to parse the error message. But as this unchecked exceptions is not meant to be caught, there is little sense in storing those parameters.
One last remark. The introduction of canAddRoll(int) reduced the expressiveness of the thrown exceptions and hence the clarity of the error message the user sees if he enters a nonsensical roll. Before the change the message would say whether the roll or the frame was out of bounds, or that the game was already complete. Now it merely states, that there was an error. This certainly is no improvement, and we later we might want to mend that.
Conclusion
The preference of unchecked exceptions over checked exceptions is not backed as much by technical considerations as by programming conventions. A checked exception cries out a warning to the programmer: Something might go wrong here, and you will have to deal with it! An unchecked exception might be overlooked, until it strikes. Some people might not feel comfortable with that.On the other hand, checked exceptions might cause a breach of the open/closed-principle. They do not improve the readability of the code over other approaches to error handling—at best! If they indicate a programming error then they might just cause unnecessary clutter to the caller code. And finally, they mandate a solution, effectively crippling the approach of preventing an error rather than detecting it. Unchecked exceptions value flexibility over safety, and this is not meant to sound spiteful. No documenting them, however, is negligence.
No comments:
Post a Comment