RFR: 8251989: Hex formatting and parsing utility [v6]

[Chris Hegarty]

On Mon, 19 Oct 2020 22:52:31 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

>> java.util.HexFormat utility:
>> 
>>  - Format and parse hexadecimal strings, with parameters for delimiter, prefix, suffix and upper/lowercase
>>  - Static factories and builder methods to create HexFormat copies with modified parameters.
>>  - Consistent naming of methods for conversion of byte arrays to formatted strings and back: formatHex and parseHex
>>  - Consistent naming of methods for conversion of primitive types: toHexDigits... and fromHexDigits...
>>  - Prefix and suffixes now apply to each formatted value, not the string as a whole
>>  - Using java.util.Appendable as a target for buffered conversions so output to Writers and PrintStreams
>>    like System.out are supported in addition to StringBuilder. (IOExceptions are converted to unchecked exceptions)
>>  - Immutable and thread safe, a "value-based" class
>> 
>> See the [HexFormat
>> javadoc](http://cr.openjdk.java.net/~rriggs/8251989-hex-formatter/java.base/java/util/HexFormat.html) for details.
>> Review comments and suggestions welcome.
>
> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Correct length of StringBuilder in formatHex;
>   Correct bug in formatHex(char[], 2, 3) and add test for subranges of char[]

Changes requested by chegar (Reviewer).

src/java.base/share/classes/java/util/HexFormat.java line 52:

> 50:  * {@link #toHexDigits(long)}, etc.
> 51:  * For conversions producing uppercase hexadecimal strings use {@link #withUpperCase()}.
> 52:  *

A previous iteration had class-level text describing the range of valid hexadecimal characters. It seems to have gotten
dropped.

src/java.base/share/classes/java/util/HexFormat.java line 46:

> 44:  * {@link #withPrefix(String)}, {@link #withSuffix(String)}, {@link #withDelimiter(String)}
> 45:  * or choice of {@link #withUpperCase()} or {@link #withLowerCase()} parameters using
> 46:  * a fluent builder style.

I'm not sure that "fluent builder style" adds anything here. In fact, I'm still searching for the `build` method! ;-)

src/java.base/share/classes/java/util/HexFormat.java line 64:

> 62:  * and {@link #formatHex(Appendable, byte[]) formatHex(Appendable, byte[])}.
> 63:  * The formatted output can be appended to {@link StringBuilder}, {@link System#out},
> 64:  * {@link java.io.Writer}, and {@link java.io.PrintStream}, all of which are {@link Appendable}.

This seems like an arbitrary list of Appendables. Suggest to restructure the sentence to make it clear that these are
just some common examples, and maybe drop a few.

src/java.base/share/classes/java/util/HexFormat.java line 68:

> 66:  * uppercase or lowercase digits, and the suffix.
> 67:  * A delimiter appears after each formatted value, except the last.
> 68:  * For conversions producing uppercase hexadecimal strings use {@link #withUpperCase()}.

I was curious why (only) uppercase is called out here, but I see now that is because lowercase is the default. It's
confusing to call out withUpperCase here, without prior context that lowercase is the default. Maybe just drop it,
since the previous sentence says that both are possible.

src/java.base/share/classes/java/util/HexFormat.java line 127:

> 125:  *
> 126:  * @implSpec
> 127:  * This class is immutable and thread-safe.

I'm curious why this is an implSpec, and not just a normative statement? HexFormat is a final class after all, there is
just one implementation.

src/java.base/share/classes/java/util/HexFormat.java line 90:

> 88:  *     assert(byteStr.equals("7f"));
> 89:  *     assert(b == byteVal);
> 90:  *

I understand why this is the case, but the example is highlighting that a roundtrip from byte is not possible :-(

-------------

PR: https://git.openjdk.java.net/jdk/pull/482