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
