Further review of java.util.HexFormat

[Raffaello Giulietti]

Hello Roger,


these are the changes I'm proposing after reviewing the code of 
j.u.HexFormat. The modified code available here
https://github.com/rgiulietti/jdk/commit/6759a25eb952ab19a045a83349d41b82cc1b07cb

In addition to other smaller, hopefully self-explanatory enhancements, 
here are the rationales for the changes.


Static field DIGITS should preferably be formatted with 16 values/line 
to ease a visual positional crosscheck with published ASCII/IsoLatin1 
tables.


Field digits is initialized with either UPPERCASE_DIGITS, 
LOWERCASE_DIGITS or digits from another instance, so it always ends up 
being either UPPERCASE_DIGITS or LOWERCASE_DIGITS.
Consequently:
* There's no need for requireNonNull() check in the (sole) private 
constructor.
* It's preferable to move the last comparison in method equals() as the 
first factor in the return statement, so it can return faster in case of 
a lower/upper mismatch. (Arrays.equals() first checks for ==, so it 
always returns fast as used in this class. It could even be replaced by 
a simple == )


Method fromHexDigits(CharSequence, int) either returns a value in the 
range [0x00..0xff] or throws.
Therefore, there's no need for the checks leading to the throwing of 
IllegalArgumentException in methods
* parseHex(CharSequence, int, int)
* parseNoDelimiter(CharSequence)
which can be simplified as a consequence.


The test for IllegalArgumentException in method parseHex(CharSequence, 
int, int), namely
string.length() < valueChars || (string.length() - valueChars) % stride != 0
can be simplified as
(string.length() - valueChars) % stride != 0

Indeed, at this point in the control flow we have
string.length() > 0  and  stride >= valueChars
Assuming string.length() < valueChars as in the left operand of || we 
then have
-stride <= -valueChars < string.length() - valueChars < 0
so
string.length() - valueChars) % stride != 0
which is the right operand of ||.
In other words, the left operand always implies the right one, adding 
nothing to it.


There's no need to check again for non-nullness in private method 
fromHexDigits(CharSequence, int). It is invoked from two places where 
the check is already performed.


Both fromHexDigits(CharSequence) and fromHexDigitsToLong(CharSequence) 
can simply invoke their 3 args counterparts.


If you prefer, I can prepare a PR once there's an issue in the bug 
system to associate the PR with.


Greetings
Raffaello