Inconsistent implementation of comparison logic

[peterlong]

The AbstractRelation.comparePropertyTo(String val) documentation states that the return value is

a negative integer, zero, or a positive integer as the property value is greater than, equal to, or less than the value represented by the argument, ignoring case considerations for strings.

This matches the implementation in that a negative number indicates that the value is greater (after) the property, a 0 indicates equality and a positive number indicates that the property is less than (before) the value.

The problem is that this does not match the convention when implementing such comparison methods.

The implementation of comparePropertyTo calls the compare method of some instance of the com.wiredmind.booleanalgebraengine.relations.comparators.Comparator interface.

The interface documents the return values of compare as:

an integer < 0 if lhs is less than rhs, 0 if they are equal, and > 0 if lhs is greater than rhs.

In this case lhs is the property and rhs is the value. In the face of it this appears to be a bug. However all the default implementation of the Comparator interface actually returns a negative number for greater than and a positive number for less than.

Anyone implementing custom Comparator classes (like myself) would assume the interface documentation could be trusted and that comparators follow the same convention as java.lang.Comparable. This would result in bugs and time wasted figuring out why this is not working as expected.

Can the AbstractRelation instance be changed to use the same convention as described in the Comparator interface which matches the convention used in the larger Java (and C/C++) world?

A added benefit of such a change is that the relation implementation like Ge would not be backward.

[wired-mind]

Peter- Thanks for pointing this out. I'll see if Chuck or Craig can get to making this change.

Eric

On Mon, Jan 20, 2014 at 11:58 AM, Peter Long notifications@github.comwrote:

The AbstractRelation.comparePropertyTo(String val) documentation states that the return value is

a negative integer, zero, or a positive integer as the property value is greater than, equal to, or less than the value represented by the argument, ignoring case considerations for strings.

This matches the implementation in that a negative number indicates that the value is greater (after) the property, a 0 indicates equality and a positive number indicates that the property is less than (before) the value.

The problem is that this does not match the convention when implementing such comparison methods.

The implementation of comparePropertyTo calls the compare method of some instance of the com.wiredmind.booleanalgebraengine.relations.comparators.Comparatorinterface.

The interface documents the return values of compare as:

an integer < 0 if lhs is less than rhs, 0 if they are equal, and > 0 if lhs is greater than rhs.

In this case lhs is the property and rhs is the value. In the face of it this appears to be a bug. However all the default implementation of the Comparator interface actually returns a negative number for greater than and a positive number for less than.

Anyone implementing custom Comparator classes (like myself) would assume the interface documentation could be trusted and that comparators follow the same convention as java.lang.Comparable. This would result in bugs and time wasted figuring out why this is not working as expected.

Can the AbstractRelation instance be changed to use the same convention as described in the Comparator interface which matches the convention used in the larger Java (and C/C++) world?

A added benefit of such a change is that the relation implementation like Ge would not be backward.

— Reply to this email directly or view it on GitHubhttps://github.com/wired-mind/BooleanAlgebraEngine/issues/1 .