Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » Rich Client Platform (RCP) » [DataBinding] IdentityWrapper violates requirements for equals
[DataBinding] IdentityWrapper violates requirements for equals [message #447927] Tue, 18 April 2006 07:19 Go to next message
Daniel Krügler is currently offline Daniel KrüglerFriend
Messages: 853
Registered: July 2009
Senior Member
The current implementation of IdentityWrapper violates the basic
requirements for equals (see
http://java.sun.com/j2se/1.4.2/docs/api/java/lang/Object.htm l#equals(java.lang.Object)),
namely

"For any non-null reference value x, x.equals(null) should return false"

because it can't handle null values. The current implementation:

public boolean equals(Object obj) {
if (obj.getClass() != IdentityWrapper.class) {
return false;
}
return o == ((IdentityWrapper) obj).o; }

should be changed to:

public boolean equals(Object obj) {
if (obj == null || obj.getClass() != IdentityWrapper.class) {
return false;
}
return o == ((IdentityWrapper) obj).o; }

to fix this issue. (Note: I'm aware that IdentityWrapper is an internal
class, but it looks like a class which might be made public in the
future, so I mentioned this point)

Greetings from Bremen,

Daniel Krügler
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #447975 is a reply to message #447927] Tue, 18 April 2006 15:25 Go to previous messageGo to next message
Brad Reynolds is currently offline Brad ReynoldsFriend
Messages: 309
Registered: July 2009
Senior Member
Daniel, feel free to log a bug to bugzilla if you think you've found a
bug. It'll probably keep it on the radar better than a post to the
newsgroup. Also if you have it feel free to attach a patch.

-brad

On 2006-04-18 01:19:29 -0600, Daniel Krügler <dsp@bdal.de> said:

> The current implementation of IdentityWrapper violates the basic
> requirements for equals (see
> http://java.sun.com/j2se/1.4.2/docs/api/java/lang/Object.htm l#equals(java.lang.Object)),
namely

"For
>
> any non-null reference value x, x.equals(null) should return false"
>
> because it can't handle null values. The current implementation:
>
> public boolean equals(Object obj) {
> if (obj.getClass() != IdentityWrapper.class) {
> return false;
> }
> return o == ((IdentityWrapper) obj).o; }
>
> should be changed to:
>
> public boolean equals(Object obj) {
> if (obj == null || obj.getClass() != IdentityWrapper.class) {
> return false;
> }
> return o == ((IdentityWrapper) obj).o; }
>
> to fix this issue. (Note: I'm aware that IdentityWrapper is an internal
> class, but it looks like a class which might be made public in the
> future, so I mentioned this point)
>
> Greetings from Bremen,
>
> Daniel Krügler
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #448207 is a reply to message #447975] Wed, 19 April 2006 06:07 Go to previous messageGo to next message
Daniel Krügler is currently offline Daniel KrüglerFriend
Messages: 853
Registered: July 2009
Senior Member
Brad Reynolds wrote:
> Daniel, feel free to log a bug to bugzilla if you think you've found a
> bug. It'll probably keep it on the radar better than a post to the
> newsgroup. Also if you have it feel free to attach a patch.

I'll do that Brad. I thought it would be better to filter potential
bugzilla entries, which actually aren't any because of reasons hidden to me.

For those interested: Bug entry is

https://bugs.eclipse.org/bugs/show_bug.cgi?id=137435

Thanks for your hint,

Daniel
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #448227 is a reply to message #448207] Wed, 19 April 2006 13:06 Go to previous messageGo to next message
Brad Reynolds is currently offline Brad ReynoldsFriend
Messages: 309
Registered: July 2009
Senior Member
I was actually wanting to try to look at all of these and understand
what you're needing but haven't had the time yet. Also 3.2 is winding
down so Boris and David are both probably really busy and might not get
to the posts anytime soon. Hopefully I didn't come off as sounding
ungrateful or that I thought posting them here was wrong, just trying
to ensure that they get addressed.

What do you mean when you say "which actually aren't any because of
reasons hidden to me?"

Thanks for logging the bugs.

-brad

On 2006-04-19 00:07:52 -0600, Daniel Krügler <dsp@bdal.de> said:

> Brad Reynolds wrote:
>> Daniel, feel free to log a bug to bugzilla if you think you've found a
>> bug. It'll probably keep it on the radar better than a post to the
>> newsgroup. Also if you have it feel free to attach a patch.
>
> I'll do that Brad. I thought it would be better to filter potential
> bugzilla entries, which actually aren't any because of reasons hidden
> to me.
>
> For those interested: Bug entry is
>
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=137435
>
> Thanks for your hint,
>
> Daniel
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #448271 is a reply to message #448227] Thu, 20 April 2006 06:37 Go to previous messageGo to next message
Daniel Krügler is currently offline Daniel KrüglerFriend
Messages: 853
Registered: July 2009
Senior Member
Brad Reynolds wrote:
> I was actually wanting to try to look at all of these and understand
> what you're needing but haven't had the time yet. Also 3.2 is winding
> down so Boris and David are both probably really busy and might not get
> to the posts anytime soon. Hopefully I didn't come off as sounding
> ungrateful or that I thought posting them here was wrong, just trying to
> ensure that they get addressed.

I absolutely understand, that time is critical now and your tone was
absolutely OK for me. As suggested in the bugzilla report, I attached a
proposed patch. I hope I did it as you usually do (I tried the "Create
patch" command the first time today and recognized some options
concerning the output format)

> What do you mean when you say "which actually aren't any because of
> reasons hidden to me?"

From a local perspective the equals implementation of IdentityWrapper is
buggy because there exists legal conditions when an NPE could arise.
But considering that this class is at least currently strictly internal,
one could argue, that the current internal use excludes situations where
the equals arguments could be null at all. Since I didn't proofed such
a critical situation in the currently existing internal source code
based on this class, I sayed "because of reasons hidden to me".

Btw.: There exists another internal inner (static) class, namely

ListenerSupport.IdentityWrapper

which is essentially a copy of the current global IdentityWrapper class,
that is, it has the same defect concerning its equals impl. May I
suggest to replace this complete class by the global (fixed)
IdentityWrapper class? I could also provide a patch, if you like.

Greetings from Bremen,

Daniel Krügler
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #448281 is a reply to message #448271] Thu, 20 April 2006 13:16 Go to previous messageGo to next message
Brad Reynolds is currently offline Brad ReynoldsFriend
Messages: 309
Registered: July 2009
Senior Member
On 2006-04-20 00:37:48 -0600, Daniel Krügler <dsp@bdal.de> said:

> Brad Reynolds wrote:
>> I was actually wanting to try to look at all of these and understand
>> what you're needing but haven't had the time yet. Also 3.2 is winding
>> down so Boris and David are both probably really busy and might not get
>> to the posts anytime soon. Hopefully I didn't come off as sounding
>> ungrateful or that I thought posting them here was wrong, just trying
>> to ensure that they get addressed.
>
> I absolutely understand, that time is critical now and your tone was
> absolutely OK for me. As suggested in the bugzilla report, I attached a
> proposed patch. I hope I did it as you usually do (I tried the "Create
> patch" command the first time today and recognized some options
> concerning the output format)

It looks good. That's the way to do it, or at least that's how I do it.

>
>> What do you mean when you say "which actually aren't any because of
>> reasons hidden to me?"
>
> From a local perspective the equals implementation of IdentityWrapper is
> buggy because there exists legal conditions when an NPE could arise.
> But considering that this class is at least currently strictly internal,
> one could argue, that the current internal use excludes situations where
> the equals arguments could be null at all. Since I didn't proofed such
> a critical situation in the currently existing internal source code
> based on this class, I sayed "because of reasons hidden to me".

Thanks for the explanation, I understand now.

>
> Btw.: There exists another internal inner (static) class, namely
>
> ListenerSupport.IdentityWrapper
>
> which is essentially a copy of the current global IdentityWrapper class,
> that is, it has the same defect concerning its equals impl. May I
> suggest to replace this complete class by the global (fixed)
> IdentityWrapper class? I could also provide a patch, if you like.

If you have the time please provide the patch and a bug report.
They're always appreciated.

>
> Greetings from Bremen,
>
> Daniel Krügler
Re: [DataBinding] IdentityWrapper violates requirements for equals [message #448287 is a reply to message #448281] Thu, 20 April 2006 14:44 Go to previous message
Daniel Krügler is currently offline Daniel KrüglerFriend
Messages: 853
Registered: July 2009
Senior Member
Brad Reynolds wrote:
>> Btw.: There exists another internal inner (static) class, namely
>>
>> ListenerSupport.IdentityWrapper
>>
>> which is essentially a copy of the current global IdentityWrapper class,
>> that is, it has the same defect concerning its equals impl. May I
>> suggest to replace this complete class by the global (fixed)
>> IdentityWrapper class? I could also provide a patch, if you like.
>
>
> If you have the time please provide the patch and a bug report. They're
> always appreciated.

Bug report and proposed patch have been provided here:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=137740

Greetings from Bremen,

Daniel Krügler
Previous Topic:Changing splash.bmp
Next Topic:ArrayIndexOutOfBoundExceptions and Dynamic Help
Goto Forum:
  


Current Time: Fri Dec 13 10:26:15 GMT 2024

Powered by FUDForum. Page generated in 0.03225 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software

Back to the top