[jr3 Microkernel] equals() not symmetric

[Michael Dürig <md...@apache.org>]

Hi,

I stumbled upon this by chance: The equals method for the classes 
ChildNodeEntriesMap and ChildNodeEntry is not symmetric. This will most 
certainly lead to subtle bugs later on.

Michael

Re: [jr3 Microkernel] equals() not symmetric

[Michael Dürig <md...@apache.org>]

On 10.2.12 11:00, Ard Schrijvers wrote:
> On Fri, Feb 10, 2012 at 11:41 AM, Stefan Guggisberg
> <st...@gmail.com>  wrote:
>> On Thu, Feb 9, 2012 at 11:45 PM, Michael Dürig<md...@apache.org>  wrote:
>>>
>>> Hi,
>>>
>>> I stumbled upon this by chance: The equals method for the classes
>>> ChildNodeEntriesMap and ChildNodeEntry is not symmetric.
>>
>> hmm, i am pretty sure they're symmetric, i.e.: a.equals(b) == b.equals(a)
>
> They look symmetric to me as well. Unless you refer to the Bucket innerclass
>
> ChildNodeEntriesMap#Bucket : This one has an invalid equals method
> breaking symmetry : Using super.equals in an equals implementation
> almost always breaks symmetry

BTW, AFIK the only way to get equals() right wrt. sub-typing is to use a 
double dispatch through a second helper method. Quite similar to the 
visitor pattern.

Michael

>
> Regards Ard
>
>>
>> or are you referring to the missing hashCode() override?
>> i admit that i've been lazy since those internal objects are not
>> intended to be used as keys for hash tables and sorts.
>>
>> but you're probably right, it doesn't hurt to implement them :)
>>
>> cheers
>> stefan
>>
>>> This will most
>>> certainly lead to subtle bugs later on.
>>>
>>> Michael
>
>
>

Re: [jr3 Microkernel] equals() not symmetric

[Michael Dürig <md...@apache.org>]

On 10.2.12 11:00, Ard Schrijvers wrote:
> On Fri, Feb 10, 2012 at 11:41 AM, Stefan Guggisberg
> <st...@gmail.com>  wrote:
>> On Thu, Feb 9, 2012 at 11:45 PM, Michael Dürig<md...@apache.org>  wrote:
>>>
>>> Hi,
>>>
>>> I stumbled upon this by chance: The equals method for the classes
>>> ChildNodeEntriesMap and ChildNodeEntry is not symmetric.
>>
>> hmm, i am pretty sure they're symmetric, i.e.: a.equals(b) == b.equals(a)
>
> They look symmetric to me as well. Unless you refer to the Bucket innerclass
>
> ChildNodeEntriesMap#Bucket : This one has an invalid equals method
> breaking symmetry : Using super.equals in an equals implementation
> almost always breaks symmetry

Yes, that's what I was referring to actually.
Michael

>
> Regards Ard
>
>>
>> or are you referring to the missing hashCode() override?
>> i admit that i've been lazy since those internal objects are not
>> intended to be used as keys for hash tables and sorts.
>>
>> but you're probably right, it doesn't hurt to implement them :)
>>
>> cheers
>> stefan
>>
>>> This will most
>>> certainly lead to subtle bugs later on.
>>>
>>> Michael
>
>
>

Re: [jr3 Microkernel] equals() not symmetric

[Stefan Guggisberg <st...@gmail.com>]

On Fri, Feb 10, 2012 at 12:00 PM, Ard Schrijvers
<a....@onehippo.com> wrote:
> On Fri, Feb 10, 2012 at 11:41 AM, Stefan Guggisberg
> <st...@gmail.com> wrote:
>> On Thu, Feb 9, 2012 at 11:45 PM, Michael Dürig <md...@apache.org> wrote:
>>>
>>> Hi,
>>>
>>> I stumbled upon this by chance: The equals method for the classes
>>> ChildNodeEntriesMap and ChildNodeEntry is not symmetric.
>>
>> hmm, i am pretty sure they're symmetric, i.e.: a.equals(b) == b.equals(a)
>
> They look symmetric to me as well. Unless you refer to the Bucket innerclass
>
> ChildNodeEntriesMap#Bucket : This one has an invalid equals method

-> ChildNodeEntriesTree#Bucket

> breaking symmetry : Using super.equals in an equals implementation
> almost always breaks symmetry

good catch!

thanks
stefan

>
> Regards Ard
>
>>
>> or are you referring to the missing hashCode() override?
>> i admit that i've been lazy since those internal objects are not
>> intended to be used as keys for hash tables and sorts.
>>
>> but you're probably right, it doesn't hurt to implement them :)
>>
>> cheers
>> stefan
>>
>>> This will most
>>> certainly lead to subtle bugs later on.
>>>
>>> Michael
>
>
>
> --
> Amsterdam - Oosteinde 11, 1017 WT Amsterdam
> Boston - 1 Broadway, Cambridge, MA 02142
>
> US +1 877 414 4776 (toll free)
> Europe +31(0)20 522 4466
> www.onehippo.com

Re: [jr3 Microkernel] equals() not symmetric

[Ard Schrijvers <a....@onehippo.com>]

On Fri, Feb 10, 2012 at 11:41 AM, Stefan Guggisberg
<st...@gmail.com> wrote:
> On Thu, Feb 9, 2012 at 11:45 PM, Michael Dürig <md...@apache.org> wrote:
>>
>> Hi,
>>
>> I stumbled upon this by chance: The equals method for the classes
>> ChildNodeEntriesMap and ChildNodeEntry is not symmetric.
>
> hmm, i am pretty sure they're symmetric, i.e.: a.equals(b) == b.equals(a)

They look symmetric to me as well. Unless you refer to the Bucket innerclass

ChildNodeEntriesMap#Bucket : This one has an invalid equals method
breaking symmetry : Using super.equals in an equals implementation
almost always breaks symmetry

Regards Ard

>
> or are you referring to the missing hashCode() override?
> i admit that i've been lazy since those internal objects are not
> intended to be used as keys for hash tables and sorts.
>
> but you're probably right, it doesn't hurt to implement them :)
>
> cheers
> stefan
>
>> This will most
>> certainly lead to subtle bugs later on.
>>
>> Michael



-- 
Amsterdam - Oosteinde 11, 1017 WT Amsterdam
Boston - 1 Broadway, Cambridge, MA 02142

US +1 877 414 4776 (toll free)
Europe +31(0)20 522 4466
www.onehippo.com

Re: [jr3 Microkernel] equals() not symmetric

[Stefan Guggisberg <st...@gmail.com>]

On Thu, Feb 9, 2012 at 11:45 PM, Michael Dürig <md...@apache.org> wrote:
>
> Hi,
>
> I stumbled upon this by chance: The equals method for the classes
> ChildNodeEntriesMap and ChildNodeEntry is not symmetric.

hmm, i am pretty sure they're symmetric, i.e.: a.equals(b) == b.equals(a)

or are you referring to the missing hashCode() override?
i admit that i've been lazy since those internal objects are not
intended to be used as keys for hash tables and sorts.

but you're probably right, it doesn't hurt to implement them :)

cheers
stefan

> This will most
> certainly lead to subtle bugs later on.
>
> Michael