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