On Mon, 20 Mar 2023 18:12:01 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Per Minborg has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Remove unused setup method
>> - Rename method in test
>> - Add copyright header
>
> src/java.base/share/classes/java/time/ZoneOffset.java line 430:
>
>> 428: public static ZoneOffset ofTotalSeconds(int totalSeconds) {
>> 429: final class Holder {
>> 430: private static final IntFunction<ZoneOffset>
>> ZONE_OFFSET_MAPPER = new ZoneOffsetMapper();
>
> Can't the ZoneOffsetMapper itself serve as a holder class? so we move this
> singleton into ZoneOffsetMapper itself.
It is possible but this keeps the mapper more local and only accessible where
it is supposed to be used. For testing purposed, it might be better to have the
class as you propose.
> test/jdk/jdk/internal/util/LazyReferenceArray/BasicLazyReferenceArrayTest.java
> line 107:
>
>> 105:
>> 106: private static IntFunction<Integer> intIdentity() {
>> 107: return i -> i;
>
> Can't this be `Integer::valueOf`?
It can. I think either works fine. Maybe a bit nicer with a method reference as
you propose.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12346#discussion_r1142972042
PR Review Comment: https://git.openjdk.org/jdk/pull/12346#discussion_r1142973744
