Bug 362272

Comment 1 Ivan Furnadjiev 2011-10-28 04:54:56 EDT

We have another hashCode collision, but this time in QxBoxDimension#hashCode. Test case:
public void testHashCode() {
  QxBoxDimensions dim1 = QxBoxDimensions.create( 1, 1, 0, 0 );
  QxBoxDimensions dim2 = QxBoxDimensions.create( 0, 25, 0, 0 );
  assertTrue( dim1.hashCode() != dim2.hashCode() );
}
As a result, ToolItem.header-overflow padding ( 0px 25px 0px 0px ) is applied as Shell-Titlebar border-radius ( 1px 1px 0px 0px ). We really have to find a solution for ThemePropertyAdapter#getKey to return unique key not based on the hashCode.

Comment 2 Ivan Furnadjiev 2011-10-28 05:24:14 EDT

We could use the java.util.UUID class for generation of unique keys. To make them shorter we could use only the last part (after the last dash). What do you think?

Comment 3 Rüdiger Herrmann 2011-10-28 05:49:33 EDT

The QxBoxDimensions#hashCode() multiplies top, left, right and bottom with same prime number (23). Wouldn't it be sufficient to use a different prime number?

Comment 4 Ivan Furnadjiev 2011-10-28 06:04:27 EDT

(In reply to comment #3)
> The QxBoxDimensions#hashCode() multiplies top, left, right and bottom with same
> prime number (23). Wouldn't it be sufficient to use a different prime number?
Probably yes, but could we solve this issue (for all Qx objects) in a generic way? We already started a discussion about proper way to create an unique keys on this bug 361122.

Comment 5 Ivan Furnadjiev 2011-10-28 06:17:56 EDT

For example looking at QxColor#hashCode, where the same pattern is used, probably there are some colors where we can run in hashCode collision too.

Comment 6 Rüdiger Herrmann 2011-10-28 10:29:46 EDT

How about using CRC32 as is now used for image hashCodes?

Comment 7 Ivan Furnadjiev 2011-10-28 10:46:40 EDT

(In reply to comment #6)
> How about using CRC32 as is now used for image hashCodes?
Why not? +1 for CRC32.

Comment 8 Ivan Furnadjiev 2011-10-28 12:40:03 EDT

QxBoxDimensions#hashCode and QxImage#hashCode now use CRC32. Changes are in CVS HEAD.

Comment 9 Ralf Sternberg 2011-10-29 05:04:23 EDT

I think with this approach we can only reduce the probability of collisions, but not really eliminate the problem. Opened bug 362390 for a proper fix.