cmpfn.{c,h} define comparison functions for many types, including floats and doubles. It defines them like so:
int doubleCmpFn(const void *p1, const void *p2) {
double v1, v2;
v1 = *((double *) p1);
v2 = *((double *) p2);
if (v1 == v2) return 0;
return (v1 < v2) ? -1 : +1;
}
IEEE floating point numbers should not be compared for equality. (Source). As it currently stands, structures that uses a CompareFn (BST and HashMap for example) are inherently implementation defined. This does not seem suitable for a library with "portable" in its name.
A simple fix would be to change v1==v2 to something like abs(v1 - v2) < DBL_EPSILON, however, this creates a scenario in which one double is, conceptually, both equal to and less/greater than another -- in other words, a partial ordering.
The description and implementation of these functions imply that a CompareFn will produce a total ordering; this is fundamentally incompatible with IEEE floating point numbers, as they are only partially orderable. Consequently, any structures which assume a total ordering (e.g. BST) could still exhibit strange behavior with the epsilon solution.
I would propose two ways of dealing with this. One would be to remove doubleCmpFn and floatCmpFn, and have types that use CompareFns throw an error if they are given floating point numbers. The other would be somewhat more complicated as it would involve creating an additional PartialCompareFn interface and implementing it for types which are partially orderable (a superset of types that are totally orderable). Incidentally this is how ordering is done in the Rust standard library, which exports two traits (somewhat analogous to Java interfaces): Ord and PartialOrd.
cmpfn.{c,h}define comparison functions for many types, includingfloats anddoubles. It defines them like so:IEEE floating point numbers should not be compared for equality. (Source). As it currently stands, structures that uses a
CompareFn(BSTandHashMapfor example) are inherently implementation defined. This does not seem suitable for a library with "portable" in its name.A simple fix would be to change
v1==v2to something likeabs(v1 - v2) < DBL_EPSILON, however, this creates a scenario in which one double is, conceptually, both equal to and less/greater than another -- in other words, a partial ordering.The description and implementation of these functions imply that a
CompareFnwill produce a total ordering; this is fundamentally incompatible with IEEE floating point numbers, as they are only partially orderable. Consequently, any structures which assume a total ordering (e.g.BST) could still exhibit strange behavior with the epsilon solution.I would propose two ways of dealing with this. One would be to remove
doubleCmpFnandfloatCmpFn, and have types that useCompareFns throw an error if they are given floating point numbers. The other would be somewhat more complicated as it would involve creating an additionalPartialCompareFninterface and implementing it for types which are partially orderable (a superset of types that are totally orderable). Incidentally this is how ordering is done in the Rust standard library, which exports twotraits (somewhat analogous to Java interfaces): Ord and PartialOrd.