RFR: 8310072: JComboBox/DisabledComboBoxFontTestAuto: Enabled and disabled ComboBox does not match in these LAFs: GTK+ [v3]

Abhishek Kumar abhiscxk at openjdk.org
Wed Apr 17 06:11:42 UTC 2024


On Wed, 17 Apr 2024 05:45:38 GMT, Tejesh R <tr at openjdk.org> wrote:

>>> 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;
>  }`

The logical || operator doesn't check second condition if first condition is true. It checks second condition only if first one is false.
So as per current logic, if RED component is not equal, it doesn't check for GREEN and BLUE component and fails.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18644#discussion_r1568274603


More information about the client-libs-dev mailing list