Le mer. 8 avr. 2020 à 14:26, Alex Herbert <alex.d.herb...@gmail.com> a écrit :
>
>
> On 08/04/2020 00:36, Gilles Sadowski wrote:
> > 2020-04-07 23:01 UTC+02:00, Alex Herbert <alex.d.herb...@gmail.com>:
> >>
> >>> On 7 Apr 2020, at 17:47, Gilles Sadowski <gillese...@gmail.com> wrote:
> >>>
> >>> Le mar. 7 avr. 2020 à 14:54, Alex Herbert <alex.d.herb...@gmail.com
> >>> <mailto:alex.d.herb...@gmail.com>> a écrit :
> >>>> On 07/04/2020 13:43, Alex Herbert wrote:
> >>>>> Fraction
> >>>>>
> >>>>> I also did a big arrangement of Fraction and BigFraction.
> >>> Thanks.
> >>>
> >>>> I noticed that hashCode is using some variant of the format used in
> >>>> Arrays.hashCode(...)
> >>>>
> >>>> I wondered if we should standardise on returning a value as if computed
> >>>> using e.g. Arrays.hashCode(new int[]{numerator, denominator}) as was
> >>>> done for Complex with its two parts.
> >>>>
> >>>> This would change in Fraction:
> >>>>
> >>>> public int hashCode() {
> >>>> return 37 * (37 * 17 + numerator) + denominator;
> >>>> }
> >>> Strange that the constant 37 * 17 was not pre-computed. ;-)
> >> Yes. Weird. It was like that in CM 3.6.1.
> >>
> >> Not knowing if one is better than the other
> > The "17" seems to come from "Effective Java" (where J. Bloch says
> > that it is arbitrary apart from being nonzero, so "1" is fine too I guess).
> >
> >> (do you just pick a small prime
> >> for the factor?) I’d just shift it to use the same method as
> >> Arrays.hashCode
> >> for consistency with Complex.
> > Fine.
> > The factor is indeed "31" in Effective Java, saying that it must be
> > odd, and that a prime is a traditional choice.
>
> hashCode was actually broken so it needed an update.
>
> These
>
> -1 / 3
>
> 1 / -3
>
> did not have the same hashCode even though they are equal. This violates
> the Object.hashCode contract.
Oops.
I overlooked that when we decided to allow both internal representations.
>
> There were two options:
>
> 1. Create the hashCode using: |numerator|, |denominator|
>
> 2. Create the hashCode using: signum, |numerator|, |denominator|
>
> I went for option 2. Thus 1/3 and -1/3 have different hash codes.
>
> The hash code computation currently matches (for Fraction f):
>
> Arrays.hashCode(new int[] {f.signum(),
> Math.abs(f.getNumerator()),
> Math.abs(f.getDenominator())});
>
> The computation is done inline without delegating to Arrays.
I'd rather keep it simple and delegate to the JDK method above.
Indeed, is there any reasonable use-case for using a fraction
as hash map key?
If not, IMO, better make the code self-documenting.
> It could be changed to:
>
> f.signum() * Arrays.hashCode(new int[] {Math.abs(f.getNumerator()),
> Math.abs(f.getDenominator())});
-0
Same rationale as above.
> The later would remove a single integer addition operation from the
> computation so probably no speed difference.
Zero difference if "hashCode()" is never called. ;-)
Gilles
> Any opinions on this? Given the signum is a 3-state flag it may be
> better moved outside the computation. This would mean that a fraction
> with the value 0 has the hashCode 0.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
