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

jshell tool: replace use of Option.get()

XMLWordPrintable

    • b123
    • generic
    • generic
    • Not verified

      From s'marks --

      From the earlier webrev,

          http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/

      if you look in src/jdk.jshell/share/classes/jdk/internal/jshell/tool/JShellTool.java, there is the following code:

      1198 int length = 0;
      1199 int first = replayableHistory.size();
      1200 while(length < Preferences.MAX_VALUE_LENGTH && --first >= 0) {
      1201 length += replayableHistory.get(first).length() + sepLen;
      1202 }
      1203 String hist = replayableHistory
      1204 .subList(first + 1, replayableHistory.size())
      1205 .stream()
      1206 .reduce( (a, b) -> a + RECORD_SEPARATOR + b)
      1207 .get();

      The problem here is that get() will throw an exception if reduce() returns an empty Optional, which it can, based on my reading of the code. Changing this to the [proposed] getWhenPresent() makes this a bit more explicit, but doesn't fix the problem.

      The usual fix is to use one of the Optional methods that handles the empty case. Instead of get(), for example, one might call orElse("default") to supply a default string value.

      But looking further, I note that the reduce() lambda is a (flawed) idiom for joining strings with a separator. In Java 8 we introduced a String.join() method which does this, avoiding the need for a stream entirely. It also handles the empty case correctly.

          String hist = String.join(RECORD_SEPARATOR,
              replayableHistory.subList(first + 1, replayableHistory.size()));

      s'marks

            ntv Nadeesh Tv
            rfield Robert Field (Inactive)
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: