Use EqualsVerifier to test equals and hashCode, modified implementati…

[tracylynne99]

Use EqualsVerifier to test equals and hashCode, modified implementations of equals and hashCode as needed to comply with EV. Including, but not limited to:

do not include transient variables in either function, 
added missing hashCode where needed, 
use Objects.equals to compare object types, 
use Double.doubleToLongBits to compare double values.

[jfree]

The removal of transient fields from the equals() tests causes some strange results. For example, add this to LegendItemTest.java, it should pass but does not:

    public void testEquals() {
        LegendItem item1 = new LegendItem("Series 1", Color.RED);
        LegendItem item2 = new LegendItem("Series 1", Color.BLUE);
        assertNotEquals(item1, item2);
    }

[tracylynne99]

The removal of transient fields from the equals() tests causes some strange results. For example, add this to LegendItemTest.java, it should pass but does not:

    public void testEquals() {
        LegendItem item1 = new LegendItem("Series 1", Color.RED);
        LegendItem item2 = new LegendItem("Series 1", Color.BLUE);
        assertNotEquals(item1, item2);
    }

I removed those fields from the equals test because I had read (from many many places) that they should not be included. Why, exactly, was a mystery, but I figured that since the origin of that advice was "Effective Java", it had to be pretty solid.

Then I read your email about the questions I'd had about the classes whose only variables were transient and thought "Oh. Yeah ok, we do need to know if those members are equal or not"

So this morning I did some extensive Google searching, and could not find the why for this generally accepted rule. And then I came across an article in InfoWorld (https://www.infoworld.com/article/2072762/object-equality.html) that said

"transient fields are not compared because the transient keyword's purpose is to prevent fields from being written during serialization; and these fields should therefore not play a part in equality testing. Otherwise, if an object is serialized and then de-serialized, then it will not be equal to itself."

Ah, but we DO serialize the transient fields! JFC just does it manually rather than rely on Java's built in serialization - I think you had said in your email that GradientPaint doesn't serialize well, so JFC has to do it manually. But the manual serialization can only be done if the variable is transient.

So, long story short (LOL too late! :-)), when I removed the transient variables from equals, I messed up! I will fix that mistake and commit the fix.

[tracylynne99]

@jfree I finally finished adding the transient fields back! (And EV'ing more classes)

I realize there's still some conflicts and I'll resolve those for sure - just let me know if this 2nd commit looks better. I reran all the JUnit tests and they pass!

[jfree]

Thanks Tracy. I didn't get a chance to review everything but so far it is looking good. If I run 'mvn clean verify' on your branch I get just one test failure:

[ERROR] CategoryPointerAnnotationTest.testEqualsHashCode:76 EqualsVerifier found a problem in class org.jfree.chart.annotations.CategoryPointerAnnotation.
-> Non-nullity: hashCode throws NullPointerException on field arrowStroke.

Do you see the same?

[tracylynne99]

Uh oh nope I missed that one. I know how to fix it tho and will do so after work 😁 today!

…

On Tue, Nov 30, 2021 at 12:48 AM David Gilbert ***@***.***> wrote: Thanks Tracy. I didn't get a chance to review everything but so far it is looking good. If I run 'man clean verify' on your branch I get just one test failure: [ERROR] CategoryPointerAnnotationTest.testEqualsHashCode:76 EqualsVerifier found a problem in class org.jfree.chart.annotations.CategoryPointerAnnotation. -> Non-nullity: hashCode throws NullPointerException on field arrowStroke. Do you see the same? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#244 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABEKTYGV6KONYCVQFU3I6BTUORXSFANCNFSM5H54L2HQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[tracylynne99]

Ok, I finally fixed CategoryPointerAnnotation. Then I resolved the conflicts in the other files, but I did so offline because I wasn't clear on how to complete the process using the website.

Ran into a bunch of other problems which turned out to be a problem in my local environment.

So hopefully it's clear where my fix/conflict resolution is! If you want me to ditch this pull request and create a brand new one so there's no conflicts I can sure do that - just let me know.

[tracylynne99]

Ok, I finally got all the conflicts resolved - this time in the correct branch. The pull request on GitHub seems to be happy now lol.

…

On Tue, Nov 30, 2021 at 12:48 AM David Gilbert ***@***.***> wrote: Thanks Tracy. I didn't get a chance to review everything but so far it is looking good. If I run 'man clean verify' on your branch I get just one test failure: [ERROR] CategoryPointerAnnotationTest.testEqualsHashCode:76 EqualsVerifier found a problem in class org.jfree.chart.annotations.CategoryPointerAnnotation. -> Non-nullity: hashCode throws NullPointerException on field arrowStroke. Do you see the same? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#244 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABEKTYGV6KONYCVQFU3I6BTUORXSFANCNFSM5H54L2HQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.