<i18n dev> RFR: 8268469: Update java.time to use switch expressions [v3]

[Patrick Concannon]

On Wed, 9 Jun 2021 16:31:10 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - 8268469: Removed excessive spacing; corrected misplaced comments
>>  - Merge remote-tracking branch 'origin/master' into JDK-8268469
>>  - Merge remote-tracking branch 'origin/master' into JDK-8268469
>>  - 8268469: Update java.time to use switch expressions
>
> src/java.base/share/classes/java/time/Month.java line 491:
> 
>> 489:             case OCTOBER   -> 274 + leap;
>> 490:             case NOVEMBER  -> 305 + leap;
>> 491:             default -> 335 + leap;
> 
> It would be better to keep `DECEMBER` here for clarity - even if only in a comment - e.g:
> 
> 
>    case NOVEMBER  -> 305 + leap;
>    // otherwise (DECEMBER)
>    default        -> 335 + leap;

Hi Daniel. Thanks for your suggestion. Comment added. Please see commit 2ae4a57

> src/java.base/share/classes/java/time/chrono/ThaiBuddhistDate.java line 334:
> 
>> 332: 
>> 333:                 default -> with(isoDate.with(field, newValue));
>> 334:             };
> 
> My preference would be to keep the imbricated switch structure: it is not obvious whether this change is correct. Have you verified that `getChronology().range(chronoField).checkValidIntValue(newValue, chronoField);` is a no op when chronoField == ERA?

I've reverted these changes as requested. Please see commit 2ae4a57

> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 4992:
> 
>> 4990:                 }
>> 4991:                 default -> throw new IllegalStateException("unreachable");
>> 4992:             };
> 
> Not sure I like the new version more than the old, as it seems to lead to more duplication.
> Maybe the 'Y' case should be tested before entering the switch - then the switch could be used to assign 
> `var field = switch (chr) {...};`

Reverted these changes as requested. Please see commit 2ae4a57

-------------

PR: https://git.openjdk.java.net/jdk/pull/4433