On 08/04/2020 14:08, Gilles Sadowski wrote:
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.
I've documented the method to state what it is doing.
Avoiding Arrays.hashCode will be faster due to lack of array
instantiation and then boundary checking on the array during
computation. In the case of BigFraction the method also avoids having to
compute the abs() of the numerator and denominator.
If you ever did need to have a hashCode then it is nicer to have a
faster method.
It you argue for Arrays.hashCode(Object[]) then you could also argue for
Arrays.equals(Object[], Object[]) in the equals method using the sign,
and absolute numerator and denominator. I think it better leave it
functionally the same but implement the method explicitly to take
advantage of knowing the internal object representation.
I stopped short of adding this implementation detail to the javadoc for
Fraction. However in Complex we mandated in the javadoc that equals and
hashCode are equivalent to using Arrays.equals/hashCode with the two
parts of the complex number.
Indeed, is there any reasonable use-case for using a fraction
as hash map key?
If not, IMO, better make the code self-documenting.
Storage in a Set<Fraction> for unique values?
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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
