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]>:
> David, > > > On 11 Jan 2016, at 10:01, David Allouche <[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 > > > > >
