RFR: 8170769 Provide a simple hexdump facility for binary data

[Vincent Ryan]

Thanks for your review Alan.

I believe I’ve addressed your concerns in this latest webrev/javadoc:
http://cr.openjdk.java.net/~vinnie/8170769/webrev.08/ <http://cr.openjdk.java.net/~vinnie/8170769/webrev.08/>
http://cr.openjdk.java.net/~vinnie/8170769/javadoc.08/api/java.base/java/util/HexFormat.html <http://cr.openjdk.java.net/~vinnie/8170769/javadoc.08/api/java.base/java/util/HexFormat.html>


> On 11 Dec 2018, at 13:04, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
> On 08/12/2018 01:18, Vincent Ryan wrote:
>> Here’s the latest version that addresses all the current open issues:
>> 
>> webrev: http://cr.openjdk.java.net/~vinnie/8170769/webrev.07/ <http://cr.openjdk.java.net/~vinnie/8170769/webrev.07/>
>> javadoc: http://cr.openjdk.java.net/~vinnie/8170769/javadoc.07/api/java.base/java/util/HexFormat.html <http://cr.openjdk.java.net/~vinnie/8170769/javadoc.07/api/java.base/java/util/HexFormat.html>
>> 
>> CSR: https://bugs.openjdk.java.net/browse/CCC-8170769 <https://bugs.openjdk.java.net/browse/CCC-8170769>
>> 
> I read through the latest javadoc corresponding to webrev.07.
> 
> Overall I think it looks very good except for dumpAsStream(ByteBuffer, fromIndex, toIndex, chunkSize, Formatter) as it's not clear if fromIndex is from the buffer position or an absolute index. If the former then there are a couple of other issues. I see Roger has touched on the advancement of the buffer position to its limit which isn't right unless all remaining bytes in the  buffer are consumer. There is also an issue with the exception as attempting to consume beyond the limit is a BufferUnderflowException. It might be simpler to replace this method with a dumpAsStream(ByteBuffer, chunkSize, Formatter) that can lazily (rather than eagerly) consume the remaining bytes. Additional overloads could be added in the future if needed.
> 
> dumpAsStream(InputStream) specifies "On return, the input stream will be at end-of-stream" but I assume this isn't right as the method is lazy.
> 
> The 3-arg dumpAsStream doesn't specify the exception/behavior for when chunkSize is <= 0.
> 
> fromString(CharSequence) may need clarification on how it behaves when the CS is a CharBuffer. Does it consume all the remaining chars in the buffer so its position be advanced to the limit? The 3-arg version is also not clear on this point.
> 
> In Formatter interface it may be useful to expand the description of the "offset" parameter as readers may not immediately understand that it's just an input for the formatted string rather than a real offset, an example might help. It also doesn't say if the offset can be specified as <= 0L or not.
> 
> A minor comment is that several places refer to a byte array as a "byte buffers. Is this deliberate or can these be replaced with "byte array" to avoid confusion. Another minor comment is that one of the dumpAsStream methods is missing @throws NPE - maybe they should all be removed and replaced with a statement in the class description.
> 
> -Alan