RFR: 7083187: Class CSS.CssValue is missing implementations of equals() and hashCode() [v10]

Prasanta Sadhukhan psadhukhan at openjdk.org
Tue May 30 17:24:06 UTC 2023


On Tue, 30 May 2023 17:07:40 GMT, Phil Race <prr at openjdk.org> wrote:

>> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Optimize fix
>
> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2219:
> 
>> 2217:         public int hashCode() {
>> 2218:             return Float.hashCode(value);
>> 2219:         }
> 
> why isn't index and lu part of this ?

It is because only the float "value" is being considered and not the `lu` units px/pt

> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2225:
> 
>> 2223:                 return val instanceof CSS.FontSize size
>> 2224:                        && Objects.equals(size.svalue, svalue);
>> 2225:         }
> 
> This is an anomaly. In all the other cases below you are using the local fields not the original parsed string.

This is because font-size attribute can have units or percentage in it, it seems like as is being tested in the regression test, 42px, 42pt so "svalue" is being used which will have 42px/42pt so we can compare the string value of 42px or 42pt otherwise we need to parse to get the integer 42 and then the units px or pt and then compared separately

> src/java.desktop/share/classes/javax/swing/text/html/CSS.java line 2920:
> 
>> 2918:         @Override
>> 2919:         public boolean equals(Object val) {
>> 2920:             return val instanceof CSS.BackgroundPosition bp
> 
> why isn't "relative" part of equals and hashCode()?

it is said "relative" is 

bit 0, horizontal relative, bit 1 horizontal relative to
        // font size, 2 vertical relative to size, 3 vertical relative to
        // font size

I was not sure if it is need to be considered in the equals/hashcode contract..

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1210587508
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1210587619
PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1210590108



More information about the client-libs-dev mailing list