go ahead and fix it..
Bye, Norman 2009/9/15 Robert Burrell Donkin <[email protected]>: > On Sun, Sep 6, 2009 at 1:57 PM, A. Rothman <[email protected]> wrote: >> >> >> 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? > > any objections? > > - robert > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
