Is actively throwing AssertionError in Java good practice?

Mathias Bader picture Mathias Bader · Dec 25, 2016 · Viewed 8.2k times · Source

Looking through Joshua Bloch's 'Effective Java - Second Edition', I stumbled upon the following code on page 152:

double apply(double x, double y) {
    switch(this) {
        case PLUS:   return x + y;
        case MINUS:  return x - y;
        case TIMES:  return x * y;
        case DIVIDE: return x / y;
    }
    throw new AssertionError("Unknown op: " + this);
}

Now what confuses me is, that the AssertionError is actively thrown. Is that considered good practice? To my understanding assertions are used to not interfeer with the code such that when the java programming is started without assertions enabled and the assert-statements are therefore not executed, the behavior doesn't change. I would be rather confused, if I'd get a AssertionException when I run a program without even having assertions enabled.

Even though I understand that the example case might happen quite often, that you analyze a couple of different options and if it is none of them, you should throw an exception.

So is it good practice to throw an AssertionException here, or would it be better to throw a different one? If so, which one would fit best? Maybe IllegalArgumentException?


Edit for clarification: My question is not about whether we should throw an Errorhere, but if we want to throw an Exception or an Error, which one should it be? And is it good practice to actively throw AssertionErrors? The documentation says Thrown to indicate that an assertion has failed, so I have the feeling that we should not actively throw it. Is that correct?


Second Edit: Clear question: Is it good practice to actively throw an AssertionError, or should that be avoided, even though it is possible? (My guess reading the docs is the latter)

Answer

dimo414 picture dimo414 · Dec 25, 2016

I would agree with Mr. Bloch here - the alternatives (IllegalArgumentException, IllegalStateException, and UnsupportedOperationException) do not properly convey the severity of the issue, and callers may erroneously try to catch and handle this case. In fact if this line is ever reached the program in question is broken, and the only sane thing to do is exit.

The point here is that enum has a finite set of values, thus it should be impossible to reach the throw line - it would only occur if the enum's definition has changed without also fixing this instance method. Throwing a RuntimeException suggests the caller made a mistake, when in fact the method (and the enum) itself is broken. Explicitly raising an AssertionError correctly indicates the invariants this method expects have been violated.

Guava has a helpful article which breaks down when to raise different types of exceptions. They write:

A conventional assertion is a check that should only fail if the class itself (that contains the check) is broken in some way. (In some cases this can extend to the package.) These can take various forms including postconditions, class invariants, and internal preconditions (on non-public methods).

An impossible-condition check is one that cannot possibly fail unless surrounding code is later modified, or our deepest assumptions about platform behavior are grossly violated. These should be unnecessary but are often forced because the compiler can't recognize that a statement is unreachable, or because we know something about the control flow that the compiler cannot deduce.

The page says an AssertionError is the recommended way to handle these cases. The comments in their Verify class also offers some useful insights about choosing exceptions. In cases where AssertionError seems too strong raising a VerifyException can be a good compromise.

As to the specific question of Error or RuntimeException, it doesn't really matter (both are unchecked and therefore will potentially travel up the call stack without being caught), but callers are more likely to attempt to recover from a RuntimeException. Crashing the application in a case like this is a feature, because otherwise we're continuing to run an application that is (at this point) demonstrably incorrect. It's certainly less likely that callers will catch and handle AssertionError (or Error or Throwable), but of course callers can do whatever they want.