[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 7083187: Class CSS.CssValue is missing implementations of equals() and hashCode() [v10]
From:       Prasanta Sadhukhan <psadhukhan () openjdk ! org>
Date:       2023-05-31 12:55:47
Message-ID: DsY7B2tJKjqAjZl6eBr_xrvzvSq8pmemQqt2PfYBxOY=.071d13e7-0d94-4252-9ac7-edf3a585fcd8 () github ! com
[Download RAW message or body]

On Wed, 31 May 2023 09:47:50 GMT, Alexey Ivanov <aivanov@openjdk.org> wrote:

> > It might looks weird but it is the one which is working.
> > For example, for `{"font-size: 42px", "font-size: 22px"}`
> > 
> > `value` is 0.0
> > `svalue` is 42px, 22px
> > `index` false
> > `lu.units` px
> > so if I check 
> > 
> > 
> > return val instanceof CSS.FontSize size
> > && value == size.value
> > && index == size.index
> > && Objects.equals(lu, size.lu);
> > 
> > 
> > 
> > it will return equals `true `even though it should not be equal which is why I \
> > used `svalue`
> 
> This is why you have to compare the entire value of `LengthUnit` as [I described \
> above](https://github.com/openjdk/jdk/pull/13405#discussion_r1211318063). 
> From a quick debugging session, `"font-size: 42px"` is parsed into the following \
> values: 
> 
> CSS$FontSize
> value = 0.0
> index = false
> svalue = "42px"
> lu = {CSS$LengthUnit@1594} "0 42.0"
> type = 0
> value = 42.0
> units = "px"
> 
> `"font-size: 22px"` is parsed into:
> 
> CSS$FontSize
> value = 0.0
> index = false
> svalue = "22px"
> lu = {CSS$LengthUnit@1594} "0 22.0"
> type = 0
> value = 22.0
> units = "px"
> 
> 
> So [the implementation I suggested \
> today](https://github.com/openjdk/jdk/pull/13405#discussion_r1211318063) for \
> `FontSize.equals` and `LengthUnit.equals` handles it correctly. 
> The one that [I suggested last \
> week](https://github.com/openjdk/jdk/pull/13405#discussion_r1200351364) which \
> avoids implementing `LengthUnit.equals` does not work because it takes into account \
> only the `units` field of the `LengthUnit` object.

OK. Thanks for the suggestion..Updated `FontSize.equals` and `LenghtUnit.equals`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13405#discussion_r1211666980


[prev in list] [next in list] [prev in thread] [next in thread]