[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);
181
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