#1406 Adding a String and a TimeSpan (or DateTime) instance as indexes into a Table fails

[Erich]

This code snippet fails for the latest 5.0 build.
The failure only happens if the String instance and the DateTime/TimeSpan instance are created from the same value (here: "1").
Adding .DateTime~new(1) instead of .TimeSpan~new(1) will fail too.
Adding the same items to an IdentityTable (instead of Table), will work.

t = .Table~new
t[1] = 0
t[.TimeSpan~new(1)] = 0

(the issue may be related to the code fixes done for [bugs:#1365])

[Rick McGuire]

This is exactly related to the fixes for 1365 and is also a reason why it is a bad idea for the the == method to ever result in an error. Looks like 1365 needs a rethink.

[Erich]

it is a bad idea for the the == method to ever result in an error

As it is the DateTime/TimeSpan/File compareTo methods, which raise the error, I suggest a fix such as

+++ CoreClasses.orx     (working copy)
@@ -2680,7 +2680,8 @@

   use strict arg other


-  .ArgUtil~validateClass("other", other, .DateTime)
+  if \other~isA(.DateTime) then
+    return (self~identityHash - other~identityHash)~sign

   othertime = other~utcDate

We also might change the DateTime/TimeSpan hashCode methods to use something like timestamp~hashcode~bitxor(hh) (where hh is e. g. '80'x for one and 'C0'x for the other class; or any other class-specific identifier) so that identical hashCodes will occur much less likely.

[Rick McGuire]

Hmmm, I see. I think some documentation is warranted for the Orderable class outlining this as a recommendation. You might also want to added a default compareTo() method that implements a default ordering so that classes that implement orderable can delegate to the interface method in case of a type mismatch.

Note also that there is currently no guarantee that the value returned by indentityHash will always be a numeric value. It might be desirable to explicitly specify that is is.

[Erich]

add a default compareTo() method that implements a default ordering

Yes, a very nice idea. If we want to enable something like if \other~isA(self~class) then forward class(super) in a compareTo() method, would we have to implement a default ordering in both Comparable and Orderable (i. e. make an actual implementation instead of an abstract method)? We really don't know whether the caller inherits from Orderable or from Comparable (or both), right?

And, thinking about the (self~identityHash - other~identityHash)~sign it seems to me we could equally just simply always return 1 (or -1) ..

[Erich]

Committed code fixes with revision [r11177]
Added test with revision [r11178]

[Erich]

Fixed added test with revision [r11187]

[Erich]

Added rexxref updates with revision [r11675]

• documents that Orderable/Comparable compareTo() is no longer ABSTRACT but a default implmentation instead
• documents the recommendation that classes inheriting from Orderable/Comparable forward to super

Note also that there is currently no guarantee that the value returned by indentityHash will always be a numeric value. It might be desirable to explicitly specify that is is.

rexxref already states that identityHash() returns a number

I believe all suggestions have been implemented/documented, except for one thing I didn't implement:
"DateTime/TimeSpan hashCode methods to use something like timestamp~hashcode~bitxor(hh) (where hh is e. g. '80'x for one and 'C0'x for the other class; or any other class-specific identifier) so that identical hashCodes will occur much less likely. "

So I'm closing this.