On Wed, 9 Jun 2021 15:41:59 GMT, Patrick Concannon <pconcan...@openjdk.org>
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.time`
> packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
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;
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?
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) {...};`
-------------
PR: https://git.openjdk.java.net/jdk/pull/4433
