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