Ok but be concrete. What will be the solution for this particular case?
Stef
Le 11/1/16 22:30, Nicolas Cellier a écrit :
Yep, a must read if you want to improve the hash methods.
2016-01-11 10:09 GMT+01:00 Sven Van Caekenberghe <[email protected]
<mailto:[email protected]>>:
David,
> On 11 Jan 2016, at 10:01, David Allouche <[email protected]
<mailto:[email protected]>> wrote:
>
> I happened to look at the new code for LayoutFrame>>#hash
>
> hash
> ^self species hash bitXor:
> (self leftFraction bitXor:
> (self leftOffset bitXor:
> (self topFraction bitXor:
> (self topOffset bitXor:
> (self rightFraction bitXor:
> (self rightOffset bitXor:
> (self bottomFraction bitXor:
> self bottomOffset)))))))
>
> This is a terrible, terrible way to compute a hash.
>
> 0 xor 0 = 1 xor 1 = 2 xor 2 = etc.
>
> NEVER compute hashes with xor, that's wrong, and it causes
horrible, hard to debug, performance bugs down the road.
Yeah, that doesn't look very good ;-)
> SequenceableCollection>>#hash looks more like a correct way of
doing it. I do not know what is the correct, efficient way to
reuse this logic in LayoutFrame>>#hash.
There is actually a 2 part book just about hashing in Smalltalk:
http://www.lulu.com/shop/andres-valloud/hashing-in-smalltalk-theory-and-practice/paperback/product-3788892.html
It is said to be very good, I sadly have not yet read it ;-)
> By the way, how do you guys do code reviews for code integrated
into the core?
'Pharo is yours' which means: if you encounter something odd, you
are encouraged to improve it. Maybe first discuss it on the ML. If
necessary create an issue. Better provide a fix.
Sven
