Yes, the equals/hashcode chapter in Effective Java includes rules about
ignoring fields that can be calculated from other fields (which applies
to most internal fields). Basically, only things in the constructors
tend to be good candidates for equals/hashcode...
...jim
On 5/3/2016 2:00 PM, Phil Race wrote:
On 04/26/2016 04:10 PM, Jim Graham wrote:
The ComponentColorModel implementation is a meaningless wrapper around
super.equals/hashCode(). Why does it not test CCM-specific fields?
It should although this looks like it is more than just running through
all the fields and testing them for equality.
eg it looks like "initScale()" should be called if necessary before
determining equality and the field "needScaleInit" is not one that
has to be directly included in the equality comparison.
-phil.
The ComponentSampleModel.hashCode() method should be upgraded based on
the recommendations in Effective Java like the other methods here.
PixelInterleavedSampleModel and BandedSampleModel also have a
meaningless override of super.equals/hashCode().
And all of these classes suffer from casting to the specific type
before verifying its class as I mentioned in the ICM.equals() review...
...jim
On 4/25/16 2:31 AM, Jayathirth D V wrote:
Hi Jim,
I have made changes to include check for class equality in base class
and use super.equals() from subclasses.
Please find updated webrev for review :
http://cr.openjdk.java.net/~jdv/8153943/webrev.02/
Change related to ColorModel is present in JDK-7107905 review.
Thanks,
Jay
-----Original Message-----
From: Jim Graham
Sent: Saturday, April 23, 2016 7:30 AM
To: Phil Race; Jayathirth D V
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] Review Request for JDK-8153943 : In
java.awt.image package some of the classes are missing hashCode() or
equals() method
This is actually a pretty nasty issue that Joe Darcy also brought up
in the CCC review.
In order to have symmetric testing of .equals(), we pretty much have
to enforce this test at all levels, including in the original
ColorModels.equals() method. If we don't enforce this in
CM.equals(), then we could run ccm.equals(othercm) and it would
return false because the class is wrong, but turning it around and
testing
othercm.equals(ccm) would succeed because it doesn't enforce the
class equality.
So, I'd recommend that CM.equals() tests getClass() == getClass() at
the base level and then all others will use super.equals() and get
the same protection. It means you can't have a subclass of CCM be
"equals" to a different subclass of CCM, but that's an unfortunate
issue with equals needing to honor symmetry... :(
...jim
On 4/20/2016 10:17 AM, Phil Race wrote:
Hi, You removed the following test in CCM.java : 2941 if
(obj.getClass() != getClass()) {
2942 return false;
2943 }
What this means is that before your change an instance of a subclass
of CCM would never be equals() to any direct instantiatation of CCM
but after your change it can be. I suspect the condition was there
on purpose.
-phil.
On 04/20/2016 05:45 AM, Jayathirth D V wrote:
Hi,
_Please review the following fix in JDK9:_
Bug : https://bugs.openjdk.java.net/browse/JDK-8153943
This is subtask of https://bugs.openjdk.java.net/browse/JDK-6588409
Webrev : http://cr.openjdk.java.net/~jdv/8153943/webrev.00/
<http://cr.openjdk.java.net/%7Ejdv/8153943/webrev.00/>
Issue : Some of the java.awt.image classes are missing either
equals() or hashCode() method.
Solution : Add missing equals() or hashCode() methods.
Thanks,
Jay
