RFR: 8343977: Convert java/awt/TextArea/TextAreaCursorTest/HoveringAndDraggingTest to main [v3]

Tejesh R tr at openjdk.org
Mon Nov 25 05:26:11 UTC 2024


On Fri, 22 Nov 2024 19:01:17 GMT, Alexey Ivanov <aivanov at 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


More information about the client-libs-dev mailing list