Mailing-List: contact dev-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Received-SPF: pass (nike.apache.org: domain of t.p.ellison@gmail.com
 designates 72.14.220.159 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:user-agent:mime-version:to:subject:references
         :in-reply-to:x-enigmail-version:content-type
         :content-transfer-encoding;
        b=Phq7rU468l/u0yrD2Jzo3usFbMU7eIkSdRfjBWixf9ino9/CgOpjZmxnMq4GtQ01tD
         6H/UkvfjUbPS2WkyUqGgaFk4UiWAki+10pDjTpYaeoyH82Qa4HOwKLePVDyrV0RQcXGE
         vpcLUsn7WTPwQNFf1Z4ywszUEuStbc1C1FVrg=
Message-ID: <4ACEEA99.4040704@gmail.com>
Date: Fri, 09 Oct 2009 08:47:37 +0100
From: Tim Ellison <t.p.ellison@gmail.com>
User-Agent: Thunderbird 2.0.0.23 (Windows/20090812)
MIME-Version: 1.0
To: dev@harmony.apache.org
Subject: Re: svn commit: r823222 -
 /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
References: <4acec9f1.47c1f10a.129c.764b@mx.google.com>
In-Reply-To: <4acec9f1.47c1f10a.129c.764b@mx.google.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On 09/Oct/2009 06:28, Ramana Polavarapu wrote:
> It appears that  Bloch suggests that we should have the following first:
>     if (!(object instanceof CredOwner)) return false;
> Then, we can skip this check:
>     if (object instanceof CredOwner) 

That is pretty much what Jesse wrote.  Ok you inverted the instance
check...but it is equivalent in performance and behavior

@Override
public boolean equals(Object object) {
    if (object == this) {
        return true;
    }
    if (!(object instanceof CredOwner)) {
        return false;
    }
    CredOwner that = (CredOwner) object;
    return principalClass.equals(that.principalClass)
        && principalName.equals(that.principalName);
}

Regards,
Tim

> -----Original Message-----
> From: Jesse Wilson [mailto:jessewilson@google.com] 
> Sent: Friday, October 09, 2009 9:00 AM
> To: dev@harmony.apache.org
> Subject: Re: svn commit: r823222 -
> /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/sec
> urity/auth/PrivateCredentialPermission.java
> 
> 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? 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?
> 
>