RFR: JDK-8264222: Use switch expression in jshell where possible [v2]
Stuart Marks
smarks at openjdk.java.net
Sat Mar 27 00:42:24 UTC 2021
On Sat, 27 Mar 2021 00:31:30 GMT, Brian Goetz <briangoetz at openjdk.org> wrote:
>> Tagir F. Valeev has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - resultTypeOf: update comment instead of splitting the case
>> - resultTypeOf: split CONSTRUCTOR and INSTANCE_INIT/STATIC_INIT case to position the comments properly
>
> Marked as reviewed by briangoetz (Reviewer).
I have some formatting questions. I also have some weak opinions on formatting, but not strong enough to make recommendations yet. (I see you wrote "Better formatting ideas are welcome" so thanks for being open to this.) I'm interested in feedback on how the code ends up _reading_, particularly from the people who will be maintaining this code. But comments from others are welcome too.
The switch (c) in `ArgTokenizer::nextToken` is pretty nicely formatted, since each case has a single constant, they're all short, and the values are also all constants that are textually short. The only thing I'd say here is to add a bit of spacing to the default case so that the arrows line up.
In `Eval::prettyExpr` switch (typeName), the switch is a bit irregular since one of the case arms has multiple constants. This makes it a bit harder to find one if you're looking for it in particular. For example, suppose you want to find out how "int" is handled. In the old code you can scan down looking at each `case` and then look at the constant right next to it. In the new code (as modified) the "int" is farther away from the `case` keyword and is a bit lost among the values, so it's harder to see. Let me see how the variations look.
Here's the modified version as proposed in the PR:
String sinit = switch (typeName) {
case "byte", "short", "int" -> "0";
case "long" -> "0L";
case "float" -> "0.0f";
case "double" -> "0.0d";
case "boolean" -> "false";
case "char" -> "'\\u0000'";
default -> "null";
};
Variation with the arrows on the same line, but lined up:
String sinit = switch (typeName) {
case "byte", "short", "int" -> "0";
case "long" -> "0L";
case "float" -> "0.0f";
case "double" -> "0.0d";
case "boolean" -> "false";
case "char" -> "'\\u0000'";
default -> "null";
};
Variation with arrows on the next line:
String sinit = switch (typeName) {
case "byte", "short", "int"
-> "0";
case "long"
-> "0L";
case "float"
-> "0.0f";
case "double"
-> "0.0d";
case "boolean"
-> "false";
case "char"
-> "'\\u0000'";
default
-> "null";
};
Variation with arrows on the same line, aligned, with case constants one per line:
String sinit = switch (typeName) {
case "byte",
"short",
"int" -> "0";
case "long" -> "0L";
case "float" -> "0.0f";
case "double" -> "0.0d";
case "boolean" -> "false";
case "char" -> "'\\u0000'";
default -> "null";
};
-------------
PR: https://git.openjdk.java.net/jdk/pull/3210
More information about the kulla-dev
mailing list