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

Andrej Golovnin andrej.golovnin at gmail.com
Sat Dec 10 21:06:06 UTC 2016

Hi Vincent,

 177     public static Stream<String> dumpToStream(byte[] bytes, int fromIndex,
 178             int toIndex) {
 179         Objects.requireNonNull(bytes);
 180         Arrays.rangeCheck(bytes.length, fromIndex, toIndex);
 182         return StreamSupport.stream(
 183             Spliterators.spliteratorUnknownSize(
 184                 new HexDumpIterator(bytes),
 185                 Spliterator.ORDERED | Spliterator.NONNULL),
 186             false);
 187     }

I think the implementation of this method is broken as it ignores the
values of the parameters fromIndex and toIndex.

Please use char literals in the lines:

251                 sb.append(" ");

262                 sb.append(" ");

279         sb.append("|");

And I would add an empty line after the lines 325 and 326.

The test should include tests for all public methods.
And I think the test should also check that all public methods
throw exceptions as described in the JavaDocs.

Best regards,
Andrej Golovnin

> On 10 Dec 2016, at 02:02, Vincent Ryan <vincent.x.ryan at oracle.com> wrote:
> Thanks to those who provided review comments.
> I have incorporated most of them and updated the webrev:
>    http://cr.openjdk.java.net/~vinnie/8170769/webrev.01/
> The main changes are to tighten up the javadoc spec and to add three additional methods
> to support byte array ranges. The total number of public methods is now 7.
> I have not added support for InputStream or ByteBuffer. I think what we have in place is
> sufficient for now and HexDump can be enhanced later if there is demand.
> The other issue is related to streams implementation and I’d like to defer that until later.
> I think the API is in good shape now but please let me know if there are any must-fix API issues.
> Otherwise I will consider the API finalised and move this forward.
> Thanks.

More information about the core-libs-dev mailing list