harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oliver Deakin <oliver.dea...@googlemail.com>
Subject Re: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Date Fri, 16 Oct 2009 13:37:41 GMT
Tim Ellison wrote:
> On 09/Oct/2009 04:29, Jesse Wilson wrote:
>   
>> On Thu, Oct 8, 2009 at 9:31 AM, <odeakin@apache.org> wrote:
>>
>>     
>>> Add null check to equals() method.
>>>       
>>
>>         // Checks two CredOwner objects for equality.
>>     
>>>         @Override
>>>         public boolean equals(Object obj) {
>>> +            if (obj == null) {
>>> +                return false;
>>> +            }
>>>             return principalClass.equals(((CredOwner) obj).principalClass)
>>>                     && principalName.equals(((CredOwner)
>>> obj).principalName);
>>>         }
>>>
>>>       
>> Does Harmony have a standard idiom for equals methods?
>>     
>
> No different to how everyone else should write them.
>
>   
>> I don't think the
>> equals() method above is particularly awesome. It can throw
>> ClassCastExceptions and performs extra casts.
>>
>> As a straw man suggestion, I propose the following control flow as our
>> standard idiom:
>>
>> @Override public boolean equals(Object object) {
>>     if (object == this) {
>>         return true;
>>     }
>>     if (object instanceof CredOwner) {
>>         CredOwner that = (CredOwner) object;
>>         return principalClass.equals(that.principalClass)
>>             && principalName.equals(that.principalName);
>>     }
>>     return false;
>> }
>>
>>
>> It seems like a natural performance, correctness and consistency win to
>> figure out a good way to implement equals() and hashCode(), and then to do
>> that everywhere in the project.
>>
>> Of course, we would first prefer to be consistent with the RI, such as
>> implementing equals to spec when it is specified (as in Set.java) or not at
>> all for reference types like AtomicInteger.
>>
>> Comments?
>>     
>
> +1 that's just how I would expect to see it written too.
>   

Agreed - improved version committed at r825888.

Regards,
Oliver

> Regards,
> Tim
>
>   

-- 
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU


Mime
View raw message