You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by od...@apache.org on 2009/10/08 18:31:36 UTC
svn commit: r823222 -
/harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Author: odeakin
Date: Thu Oct 8 16:31:35 2009
New Revision: 823222
URL: http://svn.apache.org/viewvc?rev=823222&view=rev
Log:
Add null check to equals() method.
Modified:
harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Modified: harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java?rev=823222&r1=823221&r2=823222&view=diff
==============================================================================
--- harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java (original)
+++ harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java Thu Oct 8 16:31:35 2009
@@ -375,6 +375,9 @@
// 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);
}
Re: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Posted by Oliver Deakin <ol...@googlemail.com>.
Tim Ellison wrote:
> On 09/Oct/2009 04:29, Jesse Wilson wrote:
>
>> On Thu, Oct 8, 2009 at 9:31 AM, <od...@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
Re: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Posted by Tim Ellison <t....@gmail.com>.
On 09/Oct/2009 04:29, Jesse Wilson wrote:
> On Thu, Oct 8, 2009 at 9:31 AM, <od...@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.
Regards,
Tim
Re: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Posted by Tim Ellison <t....@gmail.com>.
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, <od...@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?
>
>
RE: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Posted by Ramana Polavarapu <sr...@gmail.com>.
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)
Regards,
Ramana
-----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, <od...@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?
Re: svn commit: r823222 - /harmony/enhanced/classlib/trunk/modules/auth/src/main/java/common/javax/security/auth/PrivateCredentialPermission.java
Posted by Jesse Wilson <je...@google.com>.
On Thu, Oct 8, 2009 at 9:31 AM, <od...@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?