RFR: 8310072: JComboBox/DisabledComboBoxFontTestAuto: Enabled and disabled ComboBox does not match in these LAFs: GTK+ [v3]
Tejesh R
tr at openjdk.org
Wed Apr 17 05:49:01 UTC 2024
On Wed, 17 Apr 2024 05:17:03 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:
>> test/jdk/javax/swing/JComboBox/DisabledComboBoxFontTestAuto.java line 125:
>>
>>> 123: private static boolean isColorMatching(Color c1, Color c2) {
>>> 124: if ((c1.getRed() != c2.getRed())
>>> 125: || (c1.getBlue() != c2.getBlue())
>>
>> This condition can be optimized since the test is done for all LAF and for each this method is called twice. Instead of using ||, using && would optimized slightly. You can check for `true` where it checks for `==` and returns true. Its just a suggestion.
>
>> This condition can be optimized since the test is done for all LAF and for each this method is called twice. Instead of using ||, using && would optimized slightly. You can check for `true` where it checks for `==` and returns true. Its just a suggestion.
>
> I think current comparison with || is ok as if any of the RGB color component doesn't match, test should fail.
I meant to use `&&` so that comparisons for failure case fails faster than `||`.
Similar to this:
` if ((c1.getRed() == c2.getRed())
&& (c1.getBlue() == c2.getBlue())
&& (c1.getGreen() == c2.getGreen())) {
return true;
} else {
System.out.println(lafName + " Enabled RGB failure: "
+ c1.getRed() + ", "
+ c1.getBlue() + ", "
+ c1.getGreen() + " vs "
+ c2.getRed() + ", "
+ c2.getBlue() + ", "
+ c2.getGreen());
return false;
}`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1568256649
More information about the client-libs-dev
mailing list