Robert Burrell Donkin wrote:
On Thu, Aug 6, 2009 at 6:19 PM, A. Rothman<[email protected]> 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("m...@here") will be
equal to the String "m...@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: [email protected]
For additional commands, e-mail: [email protected]
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
