Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8308416

JavaDoc of IllegalStateException is unclear

XMLWordPrintable

    • generic
    • generic

      A DESCRIPTION OF THE PROBLEM :
      The JavaDoc of IllegalStateException states that the exception may be thrown by a method if called at an "illegal or inappropriate time[;] the Java environment or Java application is not in an appropriate state for the requested operation".

      The problem with the JavaDoc wording — as I perceive it to be — is that it may be interpreted to support pretty much any arbitrary error case because most errors in the application will be due to bad timing or bad state (somewhere) in the running application. Such fundamentalist interpretation effectively renders the IllegalStateException a "God exception".

      The JavaDoc wording is provingly a source of confusion (ref 1), and I have personally witnessed many times a developer throwing an IllegalStateException, where another type would have been more appropriate. And the problem with an overzealous misuse of the "God exception" is, of course, that it breaks the principle of least astonishment.

      Most developers throw an IllegalStateException due to class or instance state being illegal. Exceptionally (no pun intended), the state may never change and always be such that the operation can not proceed, for example, Spliterators.ArraySpliterator::getComparator (ref 2). The state may be fictiously illegal, for example, Collections.EmptyIterator::remove (ref 3). Even more rare would be throwing the exception due to the Java environment, and the only example I believe may exist (?), is Shutdown::add (ref 4).

      I deem throwing an IllegalStateException for any reason not related to the instance or class state, as inappropriate. This view supports all the aforementioned examples from the Java SE library; they are appropriate, as the verbiage "state" can be semantic; an implementation detail (fictitious, derived, et cetera), and exceptions generally, should not be thrown because of technicalities (as opposed to Error).

      For example, in the case of Shutdown::add, the VM ("Java environment") is not the class state, nor even where a shutdown hook would have been saved; the environment is a dependency whose current status is illegal (shutting down), and it is futile for the operation to proceed. I can come up with similar examples, such as having a class throw an IllegalStateException due to the status of an embedded state machine. Although examples like these may be rare; an IllegalStateException — out of lack of a more relevant/specialized type — is still the best option, thus appropriate.

      I can share an example of an inappropriate use that I just recently came across ("inappropriate" is my personal judgement).

      A server-executed, semantically decorative, configuration object of a web app — during boot — needed to access an API from another module. The accessible instance type was statically more abstract whereas the API needed was located in a subclass. As a means to circumvent a hypothetical ClassCastException in the future, this code was inserted:

      if (!(thing instanceof SomethingWeNeedAndExpect)) {
          throw new IllegalStateException();
      }

      For this configuration object, the runtime type of the thing from another module (certainly not state) would never be anything other than the expected type, unless other parts of the application's configuration changed prior to boot (certainly no timing issue; this app would always crash deterministically with an IllegalStateException).

      I would therefore argue, that the thing's runtime type is a valid assumption to make (in fact, without the expected type, the configuration object itself should not exist):

      ((SomethingWeNeedAndExpect) thing).doTheNecessary();

      I believe (shame on me) that most developers would probably agree, for the same reasons we don't usually check a reference we need for null and then throw something arbitrary different than a NullPointerException. A ClassCastException by the JVM would in this case be unobscured with a strack trace leading exactly to where the assumption is made, whereas the IllegalStateException may prompt the unfortunate application developer to reboot the application a couple of times in a desperate attempt to solve his weird "timing issue".

      Sorry for the dwelling lol. The point I wish to raise with the example is that neither case is factually wrong, as the JavaDoc has effectively rendered the IllegalStateException a "God exception", and extremism is quite common (we are all guilty). In fact, the instanceof-code was inserted by a developer with over 20 years of experience, which only stresses how important it is that we clear up the unclear JavaDoc. Perhaps purposefully to reduce the frequency of quote unquote "creative" constructs that may obfuscate and cause real-world harm?

      I will leave it up to the reviewer to decide whether what I have said so far makes sense and is duly aligned with the class author's intent. If so, I will be more than happy to try and come up with an alternative wording.


      References

      #1 https://stackoverflow.com/q/12698275/1268003

      #2 https://github.com/openjdk/jdk/blob/d22bcc813eea719b817d3d541a843594675c0ca9/src/java.base/share/classes/java/util/Spliterators.java#L1039

      #3 https://github.com/openjdk/jdk/blob/d22bcc813eea719b817d3d541a843594675c0ca9/src/java.base/share/classes/java/util/Collections.java#L4532

      #4 https://github.com/openjdk/jdk/blob/6f75dd8741f44b3012c5cab5268e89d09121e4be/src/java.base/share/classes/java/lang/Shutdown.java#L94


            Unassigned Unassigned
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: