On Fri, 22 Nov 2024 19:01:17 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Tejesh R has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated review comments > > test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java > line 62: > >> 60: .rows((int) INSTRUCTIONS.lines().count() + 2) >> 61: .columns(40) >> 62: .testUI(initialize()) > > Suggestion: > > .testUI(HoveringAndDraggingTest::initialize) > > You should pass a method reference for the `PassFailJFrame` framework to > create UI components on EDT, unless you need to create UI components on the > main thread. See [this comment explaining the > difference](https://github.com/openjdk/jdk/pull/21029#discussion_r1764872438). Yes, understood. > test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java > line 87: > >> 85: } >> 86: >> 87: static String bigString() { > > `getLongString` could be a better name… which better fits in Java guidelines > that methods are *verbs*. Updated. > test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java > line 88: > >> 86: >> 87: static String bigString() { >> 88: String s = ""; > > It's better to use `StringBuilder` instead of `String` to create a long > string. Updated. > test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java > line 89: > >> 87: static String bigString() { >> 88: String s = ""; >> 89: for (int lines = 0; ; ++lines) { > > Suggestion: > > for (int lines = 0; lines < 50; ++lines) { > > It's better to include the end condition into the `for`-loop rather than use > `if`-block to break the loop. Updated. > test/jdk/java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest.java > line 92: > >> 90: for (int symbols = 0; symbols < 100; ++symbols) { >> 91: s += "0"; >> 92: } > > You can use > [`repeat`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/String.html#repeat(int)) > method instead of the loop. > Suggestion: > > s += "0".repeat(100); > > > Since 21, `StringBuilder` also has [`repeat` > methods](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/StringBuilder.html#repeat(int,int)). Updated with StringBuilder. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1855826829 PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1855826963 PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1855827122 PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1855825273 PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1855825635
