On Tue, 21 Feb 2023 10:29:24 GMT, Claes Redestad <[email protected]> wrote:
>> Eirik Bjorsnos has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Spell fix for 'exhaustive' in comments in sun/text/resources
>
> src/java.base/share/classes/java/lang/CharacterDataLatin1.java.template line
> 142:
>
>> 140: }
>> 141: int l = ch | 0x20; // Lowercase using 'oldest ASCII trick in
>> the book'
>> 142: if ( l <= 'z' // In range a-z
>
> Suggestion:
>
> if (l <= 'z' // In range a-z
Fixed! (My IDE does not highlight this code, making it a bit harder to spot
mistakes like this)
> test/micro/org/openjdk/bench/java/lang/Characters.java line 92:
>
>> 90: @Measurement(iterations = 5, time = 1)
>> 91: @Fork(3)
>> 92: public static class Latin1CaseConversions {
>
> Not sure if qualifying this as "Latin1" is necessary, even though that's what
> you've focused on for this PR. We could easily add some codePoints outside of
> the latin1 range (now or later) without changing the test.
>
> While having a switch with some readable names is a nice touch I think we
> should additionally allow integer codePoint as-is to keep it in line with the
> outer class, e.g. `default -> Integer.parseInt(codePoint);`
You are probably right that Latin1 is a bit narrow here, removing the prefix.
I added Integer.parseInt as the default, good idea!
-------------
PR: https://git.openjdk.org/jdk/pull/12623
