harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ramana Polavarapu" <sriram...@gmail.com>
Subject RE: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Date Fri, 09 Oct 2009 05:28:11 GMT
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) 



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

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.


View raw message