RFR: 8170769 Provide a simple hexdump facility for binary data

[Vincent Ryan]

Thanks Roger. 

I believe that all but one of your concerns are addressed 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>

The remaining issue is how best to define the dumpAsStream() method that takes a ByteBuffer.
The current approach has dropped the to/from parameters, consumes all the buffer and advances
Its position to the buffer limit.
This places the burden on the caller to size the buffer appropriately.

However I also see the merit in a ‘read-only’ approach that does not advance the buffer position.

Any further thoughts?



> On 11 Dec 2018, at 16:54, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> 
> Ho Vincent,
> 
> On 12/11/2018 11:34 AM, Vincent Ryan wrote:
>> Responses in-line.
>> 
>>> 
>>>> Its really nice/necessary that examples can be copy/pasted and compile.
>>>>> 
>>>>> 
>>>>>  - dumpAsStream(ByteBuffer, from, to, chunk, Formatter) may be confusing because it
>>>>>    is using the relative methods of ByteBuffer, but the from and to offsets are within
>>>>>    the position .. limit buffer.  That should be explicit.
>>>>>    (Or consider switching to the absolute accesses in the ByteBuffer and not affect the position)
>>>> 
>>>> Is the additional javadoc line added earlier sufficient?
>>> I would bear some reinforcing that the entire remainder of the buffer is read
>>> and the from and to are indexes within that remainder.
>>> And I'm not sure that's the desirable semantics.
>>> 
>>> It would make more sense if the from bytes from the buffer are skipped
>>> and only the (to - from) bytes are dumped.  That leaves the ByteBuffer 
>>> reading only the requested bytes.  A natural usage such as:
>>>  dumpAsStream​(ByteBuffer buffer, 0, 256, 16, formatter)
>>> would dump the next 256bytes of the ByteBuffer and position would be
>>> moved by 256.
>> 
>> [As suggested by Alan]
>> How about dropping the fromIndex and toIndex parameters and requiring the caller
>> to provide a suitably sized buffer? 
>> 
>> public static Stream<String> dumpAsStream(ByteBuffer buffer, int chunkSize, Formatter formatter) {
>> 
> I'd go the other direction and make dump use absolute offset and limit.
> The current values for position and limit are readily available from the buffer.
> 
> If the dumping code is being added to perform some diagnostics then it would not
> modify the position and not disturb the existing code that is using the buffer.
> 
> Absolute is simpler to explain and implement and has fewer side effects.
> 
>> 
>>> 
>>>> 
>>>> 
>>>>> 
>>>>> - dump(byte[], OutputStream) - I don't think the example is correct, there is no reference
>>>>>   to a stream, only the PrintStream::println method, which is not static.
>>>> 
>>>> The code is just illustrative. Real values would have to be supplied for the input bytes and the
>>>> chosen print method on the output stream. Typically, that print method will be System.out::println
>>> Examples that don't compile are really confusing and annoying.
>> 
>> Updated the ‘as if’ code to:
>> 
>>      *     byte[] bytes = ...
>>      *     PrintStream out = ...
>>      *     HexFormat.dumpAsStream(bytes, 16,
>>      *         (offset, chunk, from, to) ->
>>      *             String.format("%08x  %s  |%s|",
>>      *                 offset,
>>      *                 HexFormat.toFormattedString(chunk, from, to),
>>      *                 HexFormat.toPrintableString(chunk, from, to)))
>>      *         .forEachOrdered(out::println);
>> 
>> 
> Looks fine, thanks