[OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel subclasses are missing hashCode() or equals() or both methods

[Jayathirth D V]

Hi Phil & Jim,

Collating all the changes needed:
1) I have removed both equals() & hashCode() in CCM but forgot to add the file while creating webrev. I will include changes in CCM in updated webrev.
2) Caching of hashCode value in IndexColorModel & PackedColorModel :
	We can cache hashCode value when constructor is called or when hashCode() is called for first time. What approach we have to follow?
3) Comment section of equals() method, I will update it to :
	1449      * the target object must be the same class (and not a subclass) as this
4) I will use .equals() to compare colorSpace values in CM.equals() so it will be like we are not intending ColorSpace class to never override equals() method.

Please let me know your inputs.

Thanks,
Jay

-----Original Message-----
From: Jim Graham 
Sent: Wednesday, June 01, 2016 4:14 AM
To: Phil Race
Cc: Jayathirth D V; 2d-dev at openjdk.java.net
Subject: Re: Review Request for JDK-7107905: ColorModel subclasses are missing hashCode() or equals() or both methods

I think we should use .equals() here, otherwise it looks like a recommendation that the intent is for those classes to never implement it...

			...jim

On 05/31/2016 03:31 PM, Phil Race wrote:
> I don't know of any design intent, so yes, I meant more as a practical 
> matter.
> Unless they subclass then using equals() which I thought unlikely it 
> makes no difference here.
> But it would be proof against that to use equals, unlikely to matter 
> as it might be ..
>
> -phil.
>
> On 05/31/2016 03:19 PM, Jim Graham wrote:
>>
>>
>> On 05/31/2016 02:50 PM, Phil Race wrote:
>>> On 05/31/2016 12:20 PM, Jim Graham wrote:
>>>> Hi Jay,
>>>>
>>>> You were going to remove hashCode/equals from CCM, but instead you 
>>>> simply removed it from the patch.  You still need to edit it to 
>>>> remove the existing methods.
>>>
>>> Oh yeah ! CCM is gone from the latest webrev. I expect that was not 
>>> intentional.
>>>
>>>>
>>>> Also, I'm not sure == is a good way to compare ColorSpace instances 
>>>> - Phil?
>>>
>>> Neither ColorSpace nor ICC_ColorSpace over-ride equals or hashCode 
>>> and all the predefined ones are constructed as singletons so it 
>>> seems unlikely to cause problems
>>
>> Should it use .equals() on principle, though?  Otherwise we are 
>> baking in the assumption that it doesn't implement equals() into our 
>> implementation of the CM.equals() method.  Might it some day 
>> implement equals (perhaps it isn't a practical issue today, but it 
>> might be in the future when we forget that it was omitted in this 
>> usage we create today).
>>
>> I guess what I'm asking is if it is a design feature that different 
>> objects are considered non-equal (such as an object that, for 
>> instance, has only predetermined instances that are shared by 
>> reference and reused).  I think color spaces are sort of in-between 
>> in that we define a few constants that simply get used by reference 
>> in 99% of cases, but that isn't a design feature, more like a 
>> practical issue...
>>
>>             ...jim
>