You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "A. Rothman" <am...@amichais.net> on 2009/09/06 14:57:29 UTC
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API documentation]
Robert Burrell Donkin wrote:
> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>
>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>> equals/hashcode
>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>> bugs,
>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>> course)...
>>>>>>
>>>>>>
>>>>>>
>>>>> do you have an example of the breakage?
>>>>>
>>>>>
>>>>>
>>>> From the javadoc I added:
>>>>
>>>> * Note that this implementation breaks the general contract of the
>>>> * equals method as well as its relationship with the hashcode method,
>>>> * since it can return true when compared to a String instance
>>>> * (braking the symmetry property, and allowing equal objects to have
>>>> * different hash codes).
>>>>
>>>> in other words, it can return true when compared to a String, but a
>>>> String
>>>> will always return false when compared to a non-String, and this breaks
>>>> the
>>>> contract in two different ways.
>>>>
>>>>
>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>> was ok(ish). given that the whole idea breaks a basic contract in
>>> java, this is probably just splitting hairs. i strongly dislike this
>>> design and would prefer a separate method to allow strings to be
>>> explicitly checked.
>>>
>>> in general, MailAddress has issues, and should have been modelled as
>>> an interface
>>>
>> I still think hashCode is also broken - a MailAddress("me@here") will be
>> equal to the String "ME@HERE" but they will have different hash codes.
>>
>
> agreed
>
> i'm in favour of altering the equals behaviour on MailAddress but have
> no idea about the possible impact
>
> opinions?
>
> - robert
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
>
I'm in favor too. I've found 2 usages relying on the broken behavior
(looked in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere()
and GenericListserv.service(). Both compare a MailAddress to a message
header. Actually, on closer inspection, It looks like they're broken
anyway, since they actually get the header as a String[] which means the
equals method always returns false... did this ever work? is it needed?
Anyone have other usages or opinions?
-Amichai
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Norman Maurer <no...@apache.org>.
go ahead and fix it..
Bye,
Norman
2009/9/15 Robert Burrell Donkin <ro...@gmail.com>:
> On Sun, Sep 6, 2009 at 1:57 PM, A. Rothman <am...@amichais.net> wrote:
>>
>>
>> Robert Burrell Donkin wrote:
>>
>>> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>>
>>>>>>>>
>>>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>>>> equals/hashcode
>>>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>>>> bugs,
>>>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>>>> course)...
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> do you have an example of the breakage?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> From the javadoc I added:
>>>>>>
>>>>>> * Note that this implementation breaks the general contract of the
>>>>>> * equals method as well as its relationship with the hashcode method,
>>>>>> * since it can return true when compared to a String instance
>>>>>> * (braking the symmetry property, and allowing equal objects to have
>>>>>> * different hash codes).
>>>>>>
>>>>>> in other words, it can return true when compared to a String, but a
>>>>>> String
>>>>>> will always return false when compared to a non-String, and this breaks
>>>>>> the
>>>>>> contract in two different ways.
>>>>>>
>>>>>>
>>>>>
>>>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>>>> was ok(ish). given that the whole idea breaks a basic contract in
>>>>> java, this is probably just splitting hairs. i strongly dislike this
>>>>> design and would prefer a separate method to allow strings to be
>>>>> explicitly checked.
>>>>>
>>>>> in general, MailAddress has issues, and should have been modelled as
>>>>> an interface
>>>>>
>>>>
>>>> I still think hashCode is also broken - a MailAddress("me@here") will be
>>>> equal to the String "ME@HERE" but they will have different hash codes.
>>>>
>>>
>>> agreed
>>>
>>> i'm in favour of altering the equals behaviour on MailAddress but have
>>> no idea about the possible impact
>>>
>>> opinions?
>>>
>>> - robert
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>
>>>
>>>
>>
>> I'm in favor too. I've found 2 usages relying on the broken behavior (looked
>> in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere() and
>> GenericListserv.service(). Both compare a MailAddress to a message header.
>> Actually, on closer inspection, It looks like they're broken anyway, since
>> they actually get the header as a String[] which means the equals method
>> always returns false... did this ever work? is it needed?
>>
>> Anyone have other usages or opinions?
>
> any objections?
>
> - robert
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: [mailet-api] Fix MailAddress equals? [WAS Re: Mailet API
documentation]
Posted by Robert Burrell Donkin <ro...@gmail.com>.
On Sun, Sep 6, 2009 at 1:57 PM, A. Rothman <am...@amichais.net> wrote:
>
>
> Robert Burrell Donkin wrote:
>
>> On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<am...@amichais.net> wrote:
>>
>>>>>>>
>>>>>>> 4. MailAddress.equals breaks the equals contract and the
>>>>>>> equals/hashcode
>>>>>>> relationship - I added a doc explicitly stating this to avoid future
>>>>>>> bugs,
>>>>>>> but it would be much better to fix it (without breaking james, of
>>>>>>> course)...
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> do you have an example of the breakage?
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> From the javadoc I added:
>>>>>
>>>>> * Note that this implementation breaks the general contract of the
>>>>> * equals method as well as its relationship with the hashcode method,
>>>>> * since it can return true when compared to a String instance
>>>>> * (braking the symmetry property, and allowing equal objects to have
>>>>> * different hash codes).
>>>>>
>>>>> in other words, it can return true when compared to a String, but a
>>>>> String
>>>>> will always return false when compared to a non-String, and this breaks
>>>>> the
>>>>> contract in two different ways.
>>>>>
>>>>>
>>>>
>>>> i agree about the symmetry but IIRC when i analysed the hash code it
>>>> was ok(ish). given that the whole idea breaks a basic contract in
>>>> java, this is probably just splitting hairs. i strongly dislike this
>>>> design and would prefer a separate method to allow strings to be
>>>> explicitly checked.
>>>>
>>>> in general, MailAddress has issues, and should have been modelled as
>>>> an interface
>>>>
>>>
>>> I still think hashCode is also broken - a MailAddress("me@here") will be
>>> equal to the String "ME@HERE" but they will have different hash codes.
>>>
>>
>> agreed
>>
>> i'm in favour of altering the equals behaviour on MailAddress but have
>> no idea about the possible impact
>>
>> opinions?
>>
>> - robert
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>>
>>
>
> I'm in favor too. I've found 2 usages relying on the broken behavior (looked
> in JAMES 2.3.2 only): CommandListservProcessor.checkBeenThere() and
> GenericListserv.service(). Both compare a MailAddress to a message header.
> Actually, on closer inspection, It looks like they're broken anyway, since
> they actually get the header as a String[] which means the equals method
> always returns false... did this ever work? is it needed?
>
> Anyone have other usages or opinions?
any objections?
- robert
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org