On Tue, 19 Nov 2024 05:34:20 GMT, Tejesh R <t...@openjdk.org> wrote:
>> The test is supposed to be problem listed on macos & Linux but the Problem
>> list points to the .java file instead of the .html file.
>> Hence converting
>> java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest to main which
>> would automatically resolve the issue.
>
> Tejesh R has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated review comments
Changes requested by aivanov (Reviewer).
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).
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*.
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.
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.
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)).
-------------
PR Review: https://git.openjdk.org/jdk/pull/22026#pullrequestreview-2455287698
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854512650
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854514473
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854516050
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854436611
PR Review Comment: https://git.openjdk.org/jdk/pull/22026#discussion_r1854498694
