RFR 8251989: Hex encoder and decoder utility
Raffaello Giulietti
raffaello.giulietti at gmail.com
Fri Aug 28 16:08:40 UTC 2020
Hi Roger,
I'm reading this version
http://cr.openjdk.java.net/~rriggs/webrev-hex-formatter-8251989-1/src/java.base/share/classes/java/util/Hex.java
On 2020-08-28 16:22, Roger Riggs wrote:
> Hi Raffaello,
>
> Missed sending this yesterday, the changes are in the updated webrev.
>
> On 8/23/20 10:42 AM, Raffaello Giulietti wrote:
>
> ...
>> (3) The expressions preLen > origLen - sufLen on L.492 and sufLen >
>> origLen - preLen on L.496 are equivalent. The latter is thus useless on
>> L.496 because it is always false when execution reaches here.
> true; updated to optimize the case of empty prefix and/or suffix.
>>
>>
While the lines are now numbered L.552 and L.556, this note still holds.
In other words, L.556-557
if (sufLen > origLen - preLen ||
!suffix.contentEquals(cs.subSequence(origLen - sufLen, origLen))) {
could be replaced with the simpler
if (!suffix.contentEquals(cs.subSequence(origLen - sufLen, origLen))) {
>> (7) On L.561, delimiter.isEmpty() could be removed as checkDelimiter()
>> is not invoked when the delimiter is empty. But it's perhaps safer to
>> have it.
> It could be an assert, but asserts aren't enabled always
It could be an explicit throw new AssertionError(...), but I'm not sure
it's worth the change.
Another quick note.
Complementing toHex(int value), what about exposing the additional
public char toHighHex(int value) {
return (char)digits[value >> 4 & 0xf];
}
It would useful both to replace the various ">> 4" shifts in the code
itself and externally if one isn't willing to pay for the allocation of
the String returned by toHexPair() and prefers to deal with the two
individual chars.
> Thanks for the comments,
>
> Roger
>
Has been a pleasure
Greetings
Raffaello
More information about the core-libs-dev
mailing list