On Tue, 5 Nov 2024 16:44:14 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Daniel Gredler has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Update copyright year, imports, array style, frame title > > test/jdk/java/awt/print/PrinterJob/PrintTextTest.java line 1: > >> 1: /* > > Could you make the `PrintJAText` class a nested static class inside > `PrintTextTest`? > > Yes, it requires more refactoring but this ensures no duplicate class names. > > Since `PrintJAText` extends `PrintTextTest`, I suggest moving all the > painting logic into `PrintText` class which will be a nested static class. > Then `PrintJAText` (renamed to `PrintJapaneseText` if I understand the intent > correctly) will extend `PrintText`. > > All the other logic will remain unchanged. > > Separating the painting logic from the test class itself, I believe, would > also make thing clearer. > > > In addition to the above, let's resolve IDE warnings at lines 349 and 411 (in > the current revision): > > > - byte data[] = new byte[s.length()]; > + byte[] data = new byte[s.length()]; > > - TextLayout tl = new TextLayout(s, new HashMap(), frc); > + TextLayout tl = new TextLayout(s, new HashMap<>(), frc); > > > Overall, the changes look good to me. Thank you for your contribution. Thanks! Let me know if this last commit is what you had in mind. It might be easier to review with whitespace ignored, since the class additions required some indentation changes. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21716#discussion_r1829763803
