[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] Review Request for JDK-7107905: ColorModel subclasses are missing hashCode() or
From:       Jim Graham <james.graham () oracle ! com>
Date:       2016-05-31 22:44:13
Message-ID: 574E13BD.9060701 () oracle ! com
[Download RAW message or body]

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
>
[prev in list] [next in list] [prev in thread] [next in thread]