9

I'm reading the source of XWalkUIClientInternal and I ran into the following code:

    switch(type) {
        case JAVASCRIPT_ALERT:
            return onJsAlert(view, url, message, result);
        case JAVASCRIPT_CONFIRM:
            return onJsConfirm(view, url, message, result);
        case JAVASCRIPT_PROMPT:
            return onJsPrompt(view, url, message, defaultValue, result);
        case JAVASCRIPT_BEFOREUNLOAD:
            // Reuse onJsConfirm to show the dialog.
            return onJsConfirm(view, url, message, result);
        default:
            break;
    }
    assert(false);
    return false;

I've never really seen this technique nor really thought about it before, but I guess this essentially means "this is unreachable code and should not happen ever", and crash the app no matter what. Although technically you can do that with a Throwable, as long as it's not caught.

So my question is, which one is better and why, assert(false) or throwing a RuntimeException, or maybe an Error?

EpicPandaForce
  • 79,669
  • 27
  • 256
  • 428

2 Answers2

12

The biggest difference between

assert false;

(The parenthesis are not needed, assert is not a function but a statement.) and

throw new RuntimeException();

is that the assertion can be disabled. Actually, it is disabled by default unless the JVM is started with the -ea (“enable assertions”) flag. If assertions are enabled, assert false will unconditionally throw an AssertionError which derives from Error. But since assertions can be disabled, there are two problems,

  • the error might go undetected and
  • control flow analysis requires a dummy return statement after the assert (which is mostly clutter).

Therefore, in the above case, I'd certainly go with an explicit (and more concise)

throw new AssertionError("invalid type " + type);

instead of an assert followed by a dummy return.

As mentioned in the comments, this is assuming that type is an internal parameter and an invalid value indicates a bug in the logic itself. If it is an input parameter, it should be validated according to the usual rules and an IllegalArgumentException be thrown if validation fails.

5gon12eder
  • 24,280
  • 5
  • 45
  • 92
  • And if you wanted to throw something, would you throw a `RuntimeException` so that not even `catch(Exception e)` would catch it, or a subclass of `Error`? – EpicPandaForce Feb 12 '15 at 14:38
  • 1
    You can throw an `AssertionError`, that's probably most appropriate. I have edited my answer accordingly. – 5gon12eder Feb 12 '15 at 14:40
  • Yes, you're right, that does seem like the best approach. It's more informative than just `Assertion failed: false`, and isn't caught by a mere `catch(Exception e)` branch. – EpicPandaForce Feb 12 '15 at 14:43
  • Note that you can also add a custom error message to an `assert`ion if you like. See [this answer](http://stackoverflow.com/a/2758262/1392132) for a short example. – 5gon12eder Feb 12 '15 at 14:46
  • 2
    I disagree: I would rather use `IllegalStateException` is `type` is invalid rather than `AssertionError`. Any `Error` shouldn't be unless a serious condition occurred. Besides, the Javadoc states that `AssertionError`: `Thrown to indicate that an assertion has failed.`. It's **specific** to `assert`. – Buhake Sindi Feb 12 '15 at 14:46
  • @BuhakeSindi If `type` is an input parameter, then an `IllegalArgumentException` is appropriate. But I was assuming that this is some internal code where an invalid value of `type` indicates a bug in the logic itself, not a wrong usage by the client. – 5gon12eder Feb 12 '15 at 14:48
  • I did not mention `IllegalArgumentException` but of `IllegalStateException`. Internally, `type` has a state, hence why I used it. – Buhake Sindi Feb 12 '15 at 14:50
  • @BuhakeSindi I know, but I think that an `IllegalStateException` should only be used if it's the client's fault that the object is in the wrong state. Such as requesting a solution from an equation solver without initializing it with a coefficient matrix first. From [the docs](http://docs.oracle.com/javase/7/docs/api/java/lang/IllegalStateException.html): *“Signals that a method has been invoked at an illegal or inappropriate time.”* – 5gon12eder Feb 12 '15 at 14:54
  • Not necessarily, there maybe other processes that can affect a state of a `type` but doesn't require user input. This means that the `type` is necessary for the next flow of statement executions and needs to be of correct state before they are executed. The javadoc clearly states: `Signals that a method has been invoked at an illegal or inappropriate time. In other words, the Java environment or Java application is not in an appropriate state for the requested operation.`. – Buhake Sindi Feb 12 '15 at 14:56
  • This is not good practice. AssertionErrors should be left to indicate that an `assert` statement failed. If you throw them manually, you risk polluting the program particularly in the situation that someone enables assertions (as both your code and actual `assert`s will throw AssertionErrors). Furthermore, `AssertionError` is a subclass of `Error` and is generally not good to be thrown by a developer. – Kröw Feb 23 '23 at 06:04
1

Following the Oracle guidelines (Programming with assertions), assertions was designed for testing purposes:

An assertion is a statement in the Java programming language that enables you to test your assumptions about your program. For example, if you write a method that calculates the speed of a particle, you might assert that the calculated speed is less than the speed of light.

Each assertion contains a boolean expression that you believe will be true when the assertion executes. If it is not true, the system will throw an error. By verifying that the boolean expression is indeed true, the assertion confirms your assumptions about the behavior of your program, increasing your confidence that the program is free of errors.

In your example, the developer assumed that the code never reaches the assert statement. If, by rare occurrence it does, the assert(false) will throw an Error (since it should never get there). This was done for testing purposes. So, use assert purely for testing purposes.

Buhake Sindi
  • 87,898
  • 29
  • 167
  • 228