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.r...@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.
>
