[9] RFR 8170769: Provide a simple hexdump facility for binary data

[Vincent Ryan]

Thanks for your review comments Alan.
Responses below.


> On 8 Dec 2016, at 19:44, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 07/12/2016 16:32, Vincent Ryan wrote:
> 
>> A hexdump facility has been available for many, many years via an unsupported class: sun.misc.HexDumpEncoder.
>> Although that class was always unsupported, it was still accessible. That accessibility changes with Jigsaw so I’m proposing
>> a very simple replacement in a new and supported class: java.util.HexDump.
>> 
>> Thanks.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8170769
>> Webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.00/
> I'm happy to see a proposal for an API in Java SE for this, it's always sad to bump into code that is using a utility method exposed in java.xml.bind or the JDK internal hex dumper.

Yes it’s long overdue.

> 
> I've skimmed the API. The class name and package look okay. I assume you can make HexDump final (I realize the ctor is private).

OK.

> 
> The toHexString/fromHexString methods are really useful but I think the javadoc needs to be fleshed out a bit. For example, fromHexString doesn't make it clear whether the A-F need to be in a specific case, the IAE description doesn't make it clear that it is thrown when there is invalid input. The description of toHexString can be clearer on the length of the returned string, the reader might wonder if there is a "0x" prepended for example.

OK.

> 
> I like dump(byte[]) but I could imagine calls to "customize" the returned string in many ways. After reading dumpToStream then I wonder if it might be better to drop dump and let users do their own layout. If you keep it then the javadoc needs work - the reader will have many questions. What is "non-printable", what is the character in the output that splits the lines, how is the last line handled when the input is not a multiple of 16 bytes. Given dumpToStream then I wonder if might be better to drop this method.

The behaviour of the dump method can also be achieved by manipulating the stream generated by the dumpToStream
method but that requires additional programming. The dump method does all that work for you so I’d prefer to keep it as a convenience method. 

> 
> dumpToStream(byte[]) is interesting too but again needs a lot more javadoc. I assume the spliterator will be replaced eventually as the size is known so you can do a good implementation.

Yes. Paul has provided guidance on implementation details that and I will pick that up later once this class has been integrated.

> 
> It would be useful to put in some @see references from methods like Integer.toHexString.

Where? Just on toHexString ?

> 
> I don't have comments on the implementation/test at this time.
> 
> -Alan.