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
