-
Bug
-
Resolution: Fixed
-
P4
-
9
-
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
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