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

[Alexey Ivanov]

On Wed, 31 May 2023 03:09:12 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> I already told using your suggested code fails the regression test..

Which regression test do you refer to? The one for `AttributeSet.equals` or another?

I can't see how implementing `equals` could affect other tests… unless some tests already use `AttributeSet.equals`.

If the test being developed fails, it means the implementation in `CSS` is wrong.

I haven't been able to thinker with the code myself.

> What are you suggesting then?

Neither parsing the string nor using the raw string seems right. The object already contains the parsed data as a set of fields: `value`, `index`, `lu`.
https://github.com/openjdk/jdk/blob/927a9ed68371597eba1609f97ac03dd1de812e26/src/java.desktop/share/classes/javax/swing/text/html/CSS.java#L2203-L2205

I think the fields should be used in the implementation of the `equals` method, and, as [I already noted](https://github.com/openjdk/jdk/pull/13405#discussion_r1200351364), it should look something like this:

        @Override
        public boolean equals(Object val) {
            return val instanceof CSS.FontSize size
                   && value == size.value
                   && index == size.index
                   && Objects.equals(lu, size.lu);
        }


Using `Objects.equals` handles `null` values in `lu` fields gracefully. At the same time, this requires that `LengthUnit` implements `equals`. I suggested an implementation without `LengthUnit.equals`, it was probably incorrect because I didn't test it; however, implementing `LengthUnit.equals` is a better solution.


        @Override
        public boolean equals(Object obj) {
            return obj instanceof LengthUnit ou
                   && type == ou.type
                   && value == ou.value
                   && Objects.equals(units, ou.units);
        }


(Okay, I thought `CSS.LengthValue` also uses `LengthUnit` class but it doesn't, or rather it doesn't have it as a field, units are stored as a `String` value there.)

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

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