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