[OpenJDK 2D-Dev] Review Request for JDK-8153943 : In java.awt.image package some of the classes are missing hashCode() or equals() method

[Jim Graham]

The ComponentColorModel implementation is a meaningless wrapper around 
super.equals/hashCode().  Why does it not test CCM-specific fields?

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 at 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
>>>
>>>
>>>
>>