On Fri, 6 Oct 2023 17:19:29 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR which cleans up the static test utility class
> _HexDumpReader_.
>
> This cleans up the code by replacing the nested _ByteArrayBuilder_ class with
> _HexFormat_, and simplifies the File processing by using a stream. Changes
> were tested to ensure that the _text_ tests are still getting equivalent
> ByteArrayInputStreams as before.
test/jdk/java/text/testlib/HexDumpReader.java line 60:
> 58: } catch (IOException e) {
> 59: throw new RuntimeException(String.format("Utility class
> HexDumpReader" +
> 60: " threw %s when trying to read the file %s", e,
> hexFile.getName()));
It's OK as it is. It could simply throw that `IOException` which may be simpler.
test/jdk/java/text/testlib/HexDumpReader.java line 65:
> 63: String hexString = lines.stream().map(String::trim)
> 64: .map(s -> (s.contains("#")) ? s.substring(0,
> s.indexOf("#")).trim() : s)
> 65: .filter(s -> !s.isEmpty()).collect(Collectors.joining());
I think the common style is to wrap the line per each stream operation. Also,
`trim()` can be done after removing `#` so that trimming is not necessary in
that map operation.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16075#discussion_r1349215609
PR Review Comment: https://git.openjdk.org/jdk/pull/16075#discussion_r1349210970
