On Tue, 25 May 2021 10:31:33 GMT, Patrick Concannon <[email protected]>
wrote:
>> Tagir F. Valeev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Indent some lines to make GitHub diff happier
>
> src/java.base/share/classes/java/util/GregorianCalendar.java line 1730:
>
>> 1728: int value = -1;
>> 1729: switch (field) {
>> 1730: case MONTH -> {
>
> HI @amaembo, thanks for taking a look at this.
>
> I think you should be careful here, and ask is introducing the enchanced
> switch adding any value? While I think the enchanced switch can be valuable
> when it makes the code more readable, it shouldn't be introduced just for the
> sake of using it.
@pconcannon thank you for review!
In this particular place, it makes the code more better in the following points:
- It removes `break;` operators at the end of each branch, which add nothing
but a visual noise
- It makes the whole switch shorter by 22 lines
- Using `->` syntax clearly states that no branch in this switch has a
fall-through behavior, so the code reader should not check every branch to
ensure this. Just see very first `->` and you already know that no fallthrough
is here.
- In case if new branches will appear in the future, the new-style switch will
protect future maintainers from accidental fall-through, an error that often
happens with old-style switches.
> src/java.base/share/classes/java/util/PropertyPermission.java line 337:
>
>> 335: */
>> 336: static String getActions(int mask) {
>> 337: return switch (mask & (READ | WRITE)) {
>
> Just a suggestion, but it might be more readable if you align the lambda
> operators
Thank you for pointing to this. Actually, I just noticed that there was some
kind of vertical alignment before my change. I added vertical alignment for
single-line switch rules.
-------------
PR: https://git.openjdk.java.net/jdk/pull/4161
