groovy
  1. groovy
  2. GROOVY-2503 MOP 2.0 design inflluencing issues
  3. GROOVY-3364

== operator does not work anymore if Comparable is implemented!

    Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5.7, 1.6-rc-3
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
      None
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      0

      Description

      Once a class implements Comparable, the == operator does not work anymore!

      class X implements Comparable {
        int compareTo(Object object) {
          return 0
        }
        boolean equals(Object object) {
          return true;
        }
      }
      
      public void testEqualityOperator() {
        def x = new X()
        def y = new Y()
        assert x == x   // OK 
        assert x == y // OK 
        assert x == 1 // FAIL - Neither compareTo() nor equals() is called!
      }
      

      The location of the bug is in

      DefaultTypeTransform.compareToWithEqualityCheck(Object left, Object right, boolean equalityCheckOnly):
      
              if (equalityCheckOnly) {
                  return -1; // anything other than 0
              }
      

      The fix is:

              if (equalityCheckOnly) {
                return ((Boolean) InvokerHelper.invokeMethod(left, "equals", right)).booleanValue() ? 0 : -1;
              }
      

      An even better fix would be to always use equals for the == operator - but this is already discussed in other issues.

      I've set this to a BLOCKER beause this issue leads to the fact that I cannot use Groovy for relaxed typing - and that makes it useless for many DSLs!

        Activity

        Hide
        blackdrag blackdrag added a comment -

        I added some code text to make the issue more readable.

        Then about the issue itself... we have here a case where the left side is implementing Comparable, but the right side is not... that also means that left and right implement different types, unrelated ones on top of that. Currently we have defined, that if the left side is a Comparable and the right side is not, that those two can never be equal. It is kind of strange to go to compareTo, but then to fall back to equals. On top of that a usual implementation of equals returns false if the types are not equal enough... comparing itself with an Integer in equals will lead in any normal java class to false.

        If anything then I would mark this report as improvement, because you want to "advance" the current logic. But frankly I would like to know more about your use case, because you are doing things here, that are a bit "out of spec".

        Also I wonder why the issue got this many votes, when I hear the first time about that part of the logic being a problem. So I guess they would have some reasons for having such a change and I would like to hear these

        Show
        blackdrag blackdrag added a comment - I added some code text to make the issue more readable. Then about the issue itself... we have here a case where the left side is implementing Comparable, but the right side is not... that also means that left and right implement different types, unrelated ones on top of that. Currently we have defined, that if the left side is a Comparable and the right side is not, that those two can never be equal. It is kind of strange to go to compareTo, but then to fall back to equals. On top of that a usual implementation of equals returns false if the types are not equal enough... comparing itself with an Integer in equals will lead in any normal java class to false. If anything then I would mark this report as improvement, because you want to "advance" the current logic. But frankly I would like to know more about your use case, because you are doing things here, that are a bit "out of spec". Also I wonder why the issue got this many votes, when I hear the first time about that part of the logic being a problem. So I guess they would have some reasons for having such a change and I would like to hear these
        Hide
        Peter Rietzler added a comment -

        The use case is a DSL with "relaxed" type handling. Our DSL user is a power user, but does not have programming skills. Therefore I don't want him to care about type convertions, e.g. "x > 50" should work if x is a String that smells like a Number. My first approach was to have some kind of "AutoType" class that wraps around another type and provides the conversions on the fly by overriding various operators. Currently I can either implement Comparable or override equals - but not both ...

        By the way - The Java documentation does not require Comparable.compareTo to correspond with equals - here is a snippet from the Comparable.compareTo JavaDoc:

        "It is strongly recommended, but not strictly required that (x.compareTo==0) == (x.equals). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

        Show
        Peter Rietzler added a comment - The use case is a DSL with "relaxed" type handling. Our DSL user is a power user, but does not have programming skills. Therefore I don't want him to care about type convertions, e.g. "x > 50" should work if x is a String that smells like a Number. My first approach was to have some kind of "AutoType" class that wraps around another type and provides the conversions on the fly by overriding various operators. Currently I can either implement Comparable or override equals - but not both ... By the way - The Java documentation does not require Comparable.compareTo to correspond with equals - here is a snippet from the Comparable.compareTo JavaDoc: "It is strongly recommended, but not strictly required that (x.compareTo ==0) == (x.equals ). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."
        Hide
        blackdrag blackdrag added a comment -

        first of "x > 50" is not "x == 50" and even if that works, what about "50 < x"? Once the order is reversed you will fail with your DSL... and not only with compareTo, but with equals too.

        The operator logic is still a open problem I will try to access in 2.0, but till then it might be more easy if you do your own thing here. You could hook into the compiler and reaplace every BinaryExpression for <=>,<,<=,==,>=,> and != with your own method and so bypass all the stuff Groovy is doing.

        Show
        blackdrag blackdrag added a comment - first of "x > 50" is not "x == 50" and even if that works, what about "50 < x"? Once the order is reversed you will fail with your DSL... and not only with compareTo, but with equals too. The operator logic is still a open problem I will try to access in 2.0, but till then it might be more easy if you do your own thing here. You could hook into the compiler and reaplace every BinaryExpression for <=>,<,<=,==,>=,> and != with your own method and so bypass all the stuff Groovy is doing.
        Hide
        Peter Rietzler added a comment -

        let's ignore my use case for the moment - I still stick to my opinion that this issue is a bug and not an improvement. Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo. Maybe this issue should be set as duplicate to #GROOVY-2334 since the resolution of #GROOVY-2334 would resolve this issue too.

        Show
        Peter Rietzler added a comment - let's ignore my use case for the moment - I still stick to my opinion that this issue is a bug and not an improvement. Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo. Maybe this issue should be set as duplicate to # GROOVY-2334 since the resolution of # GROOVY-2334 would resolve this issue too.
        Hide
        blackdrag blackdrag added a comment -

        If I mark this as duplicate of GROOVY-2334 then this issue won't be resolved before Groovy 2.0 and we won't try to find an alternative solution anymore.

        You wrote: "Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo."

        Wouldn't you find it strange that <,<=,=>,>,<=> all use compareTo, but == does not? Normally you assume that x<=y & x>=y implies x==y. But if equals and compareTo differ for (x,y), then this won't be the case.

        Show
        blackdrag blackdrag added a comment - If I mark this as duplicate of GROOVY-2334 then this issue won't be resolved before Groovy 2.0 and we won't try to find an alternative solution anymore. You wrote: "Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo." Wouldn't you find it strange that <,<=,=>,>,<=> all use compareTo, but == does not? Normally you assume that x<=y & x>=y implies x==y. But if equals and compareTo differ for (x,y), then this won't be the case.
        Hide
        Peter Rietzler added a comment -

        I absolutely agree with you and I understand your catch-22. If I were to decide, I'd stick with Java and would use equals() for ==. The Java developer's mindset for equality is equals(). If you change this contract than Java objects behave differently in Groovy, and even within Groovy there are some other strange side effects - in this example, the second assertion will fail (because BigDecimal has a natural ordering that's inconsistent with equals):

        def v1 = 1.25
        def v2 = 1.250
        def values = [v1]
        assert v1 == v2
        assert values.contains(v2)
        

        Inconsistent natural ordering is a rare case - the only really prominent example I know is BigDecimal (see javadoc of Comparable and BigDecimal) - and since Groovy makes heavy use of BigDecimals this has some potential to break existing code!

        For the meantime I'd apply the following patch because neither Object.equals() nor Comparable.compareTo() require the object to be compared to be of a special type or to be within the type's hierarchy (see JavaDoc). Here is a snippet of DefaultTypeTransform.compareToWithEqualityCheck:

                ...
                    if (!equalityCheckOnly || left.getClass().isAssignableFrom(right.getClass())
                            || right.getClass().isAssignableFrom(left.getClass())
                            || (left instanceof GString && right instanceof String)) {
                        Comparable comparable = (Comparable) left;
                        return comparable.compareTo(right);
                    }
                }
        
                if (equalityCheckOnly) {
                    return -1; // anything other than 0
                }
                throw new GroovyRuntimeException("Cannot compare " + left.getClass().getName() + " with value '" +
                        left + "' and " + right.getClass().getName() + " with value '" + right + "'");
        

        Currently the various compare-operators do not catch the behavior of all Java classes (especially for Java classes that want to be comparable to other types). There are cases (such as mine) where I really want this behavior (e.g. think of some kind of relaxed type comparison adapter). A quick patch could look like this:

        ...
        try {
          Comparable comparable = (Comparable) left;
          return comparable.compareTo(right);
        } catch(ClassCastException ex) {
          if(equalityCheckOnly)
            return ((Boolean) InvokerHelper.invokeMethod(left, "equals", right)).booleanValue() ? 0 : -1;
          throw new GroovyRuntimeException("Cannot compare " + left.getClass().getName() + " with value '" +
            left + "' and " + right.getClass().getName() + " with value '" + right + "'");
        }
        

        This would keep your current Groovy contract to use compareTo() if the object is Comparable and use equals() otherwise. This will however, not solve the BigDecimal example shown above and still is inconsistent with Java equality. If currently affected Groovy code should be brought to a minium (I'd guess 99% unaffected), then you have to replace the InvokeHelper call with a return of -1.

        Show
        Peter Rietzler added a comment - I absolutely agree with you and I understand your catch-22. If I were to decide, I'd stick with Java and would use equals() for ==. The Java developer's mindset for equality is equals(). If you change this contract than Java objects behave differently in Groovy, and even within Groovy there are some other strange side effects - in this example, the second assertion will fail (because BigDecimal has a natural ordering that's inconsistent with equals): def v1 = 1.25 def v2 = 1.250 def values = [v1] assert v1 == v2 assert values.contains(v2) Inconsistent natural ordering is a rare case - the only really prominent example I know is BigDecimal (see javadoc of Comparable and BigDecimal) - and since Groovy makes heavy use of BigDecimals this has some potential to break existing code! For the meantime I'd apply the following patch because neither Object.equals() nor Comparable.compareTo() require the object to be compared to be of a special type or to be within the type's hierarchy (see JavaDoc). Here is a snippet of DefaultTypeTransform.compareToWithEqualityCheck: ... if (!equalityCheckOnly || left.getClass().isAssignableFrom(right.getClass()) || right.getClass().isAssignableFrom(left.getClass()) || (left instanceof GString && right instanceof String)) { Comparable comparable = (Comparable) left; return comparable.compareTo(right); } } if (equalityCheckOnly) { return -1; // anything other than 0 } throw new GroovyRuntimeException("Cannot compare " + left.getClass().getName() + " with value '" + left + "' and " + right.getClass().getName() + " with value '" + right + "'"); Currently the various compare-operators do not catch the behavior of all Java classes (especially for Java classes that want to be comparable to other types). There are cases (such as mine) where I really want this behavior (e.g. think of some kind of relaxed type comparison adapter). A quick patch could look like this: ... try { Comparable comparable = (Comparable) left; return comparable.compareTo(right); } catch(ClassCastException ex) { if(equalityCheckOnly) return ((Boolean) InvokerHelper.invokeMethod(left, "equals", right)).booleanValue() ? 0 : -1; throw new GroovyRuntimeException("Cannot compare " + left.getClass().getName() + " with value '" + left + "' and " + right.getClass().getName() + " with value '" + right + "'"); } This would keep your current Groovy contract to use compareTo() if the object is Comparable and use equals() otherwise. This will however, not solve the BigDecimal example shown above and still is inconsistent with Java equality. If currently affected Groovy code should be brought to a minium (I'd guess 99% unaffected), then you have to replace the InvokeHelper call with a return of -1.
        Hide
        blackdrag blackdrag added a comment -

        I think we both agree, that Comparable and equals are not a perfect fit to express all the operations we want to do with them. One problem here is if the given types differ. now javadoc for Comparable says

        This interface imposes a total ordering on the objects of each class that implements it. This ordering is referred to as the class's natural ordering, and the class's compareTo method is referred to as its natural comparison method.

        This natural order implies that the objects compare are of the same class in some sort of way or that the operation will cause an exception. Throwing an exception means the operation is not defined, so we are talking here about a partial function when you look at all possible values.
        the goal is to get a function where the signum of x.compareTo(y) is the revers version of y.compareTo(x). So if x.compareTo(y) throws an exception, y.compareTo(x) should do the same or you break symmetry big times. That includes for example that x.compareto(null) in java has to throw a NPE, because null.compareTo(x) would do that. This also implies that if x and y are of different types and if x does not know the type of y in any way, that x.compareTo(y) must throw an exception, since both types are not comparable. Which then again implies that y.compareTo(x) must throw an exception too, even if y would know x. And that is a difference from equals, which can always return false then. And the common way to ensure this is to throw a ClassCastException.
        So even if the javadoc does not say you have to use the same type, if you want to fulfill their implications, you have to do that. In the jdk source you will most probably not find an example that allows you to compare two instances of different unrelated classes. I mean they had their reason to define Comparable as "interface Comparable<T>"

        Now or your patch... true, compareTo might throw a ClassCastException, but who ensures, that is not due to a programming error in a method called from there somewhere deep down the stack? In fact we don't know if the exception is thrown as part of the contract for Comparable, or because of a different reason. So the danger is that we hide a problem, leading to endless debugging sessions and angry users later.

        Also your assumption may be correct, that most code won't be affected by this change, but only if they don't depend on the exception being thrown. So your patch builds on what you expect others not to do?

        Show
        blackdrag blackdrag added a comment - I think we both agree, that Comparable and equals are not a perfect fit to express all the operations we want to do with them. One problem here is if the given types differ. now javadoc for Comparable says This interface imposes a total ordering on the objects of each class that implements it. This ordering is referred to as the class's natural ordering, and the class's compareTo method is referred to as its natural comparison method. This natural order implies that the objects compare are of the same class in some sort of way or that the operation will cause an exception. Throwing an exception means the operation is not defined, so we are talking here about a partial function when you look at all possible values. the goal is to get a function where the signum of x.compareTo(y) is the revers version of y.compareTo(x). So if x.compareTo(y) throws an exception, y.compareTo(x) should do the same or you break symmetry big times. That includes for example that x.compareto(null) in java has to throw a NPE, because null.compareTo(x) would do that. This also implies that if x and y are of different types and if x does not know the type of y in any way, that x.compareTo(y) must throw an exception, since both types are not comparable. Which then again implies that y.compareTo(x) must throw an exception too, even if y would know x. And that is a difference from equals, which can always return false then. And the common way to ensure this is to throw a ClassCastException. So even if the javadoc does not say you have to use the same type, if you want to fulfill their implications, you have to do that. In the jdk source you will most probably not find an example that allows you to compare two instances of different unrelated classes. I mean they had their reason to define Comparable as "interface Comparable<T>" Now or your patch... true, compareTo might throw a ClassCastException, but who ensures, that is not due to a programming error in a method called from there somewhere deep down the stack? In fact we don't know if the exception is thrown as part of the contract for Comparable, or because of a different reason. So the danger is that we hide a problem, leading to endless debugging sessions and angry users later. Also your assumption may be correct, that most code won't be affected by this change, but only if they don't depend on the exception being thrown. So your patch builds on what you expect others not to do?
        Hide
        Peter Rietzler added a comment - - edited

        You are right - Catching the ClassCastException is not a good idea because there may be code that relies on the exception. BTW: I've written the patch in a way with the intention of altering existing behavior as less as possible! Note that if there is code relying on a ClassCastException to be thrown directly by compareTo (and not down the stack) - the chances that this ClassCastException is redirected to the client is nearly 0% (due to the isAssignableFrom() checks that are done before) - In this case the client would receive a GroovyRuntimeException.

        At the moment I don't see a problem with all the stuff you've written above. Sure - In Java I'd have the problem that I cannot assure that x.compareTo(y) is the revers version of y.compareTo(x) if I do not have control of both classes. However, it's Groovys strength to do such things ... I can add the required methods as I like. And I can implement Comparable<Object> if I want to.

        If you want the original exception to be delivered, then the current implementation needs to be changed (the isAssignableFrom() checks need to be eliminated and a possible ClassCastException is delivered from the compareTo method itself). You could remove the equalityCheckOnly stuff and just rely on the compareTo method. This has the side-effect that the "==" operator may throw a ClassCastException, however, since your current contract is to use Comparable if implemented, this is ok. This would be a better approach than my patch is - however - it changes behavior.

        Show
        Peter Rietzler added a comment - - edited You are right - Catching the ClassCastException is not a good idea because there may be code that relies on the exception. BTW: I've written the patch in a way with the intention of altering existing behavior as less as possible! Note that if there is code relying on a ClassCastException to be thrown directly by compareTo (and not down the stack) - the chances that this ClassCastException is redirected to the client is nearly 0% (due to the isAssignableFrom() checks that are done before) - In this case the client would receive a GroovyRuntimeException. At the moment I don't see a problem with all the stuff you've written above. Sure - In Java I'd have the problem that I cannot assure that x.compareTo(y) is the revers version of y.compareTo(x) if I do not have control of both classes. However, it's Groovys strength to do such things ... I can add the required methods as I like. And I can implement Comparable<Object> if I want to. If you want the original exception to be delivered, then the current implementation needs to be changed (the isAssignableFrom() checks need to be eliminated and a possible ClassCastException is delivered from the compareTo method itself). You could remove the equalityCheckOnly stuff and just rely on the compareTo method. This has the side-effect that the "==" operator may throw a ClassCastException, however, since your current contract is to use Comparable if implemented, this is ok. This would be a better approach than my patch is - however - it changes behavior.
        Hide
        blackdrag blackdrag added a comment -

        are you sure about the GroovyRuntimeException? These are usually carriers of real exceptions and they get unwrapped once you enter Groovy code again. Before 1.0 Groovy had a very bad tradition of swallowing exceptions and it took really a huge effort to remove all those black holes. So you may understand, that I am not eager to add a new hole.

        It is very well possible that due to a programming error a ClassCastException will occur. For a programming language it is very important to not to swallow such exceptions. Because that will mean the user will not see the place of error, instead the application will just react strangely. That is really a hell to debug.

        Because we check with isAssignableFrom it is even more a high probability, that we have a programming error if this exceptions happens.

        Show
        blackdrag blackdrag added a comment - are you sure about the GroovyRuntimeException? These are usually carriers of real exceptions and they get unwrapped once you enter Groovy code again. Before 1.0 Groovy had a very bad tradition of swallowing exceptions and it took really a huge effort to remove all those black holes. So you may understand, that I am not eager to add a new hole. It is very well possible that due to a programming error a ClassCastException will occur. For a programming language it is very important to not to swallow such exceptions. Because that will mean the user will not see the place of error, instead the application will just react strangely. That is really a hell to debug. Because we check with isAssignableFrom it is even more a high probability, that we have a programming error if this exceptions happens.
        Hide
        Peter Rietzler added a comment -

        what's your proposal to resolve the issue ?

        Show
        Peter Rietzler added a comment - what's your proposal to resolve the issue ?
        Hide
        blackdrag blackdrag added a comment -

        I don't think this is something we can solve before 2.0, so I am setting a new fix version

        Show
        blackdrag blackdrag added a comment - I don't think this is something we can solve before 2.0, so I am setting a new fix version
        Hide
        Jim Moore added a comment -

        Still an issue in Groovy 2. Any idea on if this is on the roadmap?

        Show
        Jim Moore added a comment - Still an issue in Groovy 2. Any idea on if this is on the roadmap?
        Hide
        Bertrand Francizod added a comment - - edited

        Peter Rietzler added a comment - 23/Feb/09 3:02 AM

        let's ignore my use case for the moment - I still stick to my opinion that this issue is a bug and not an improvement. Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo. Maybe this issue should be set as duplicate to #GROOVY-2334 since the resolution of #GROOVY-2334 would resolve this issue too.

        ---------

        I fully agree with this comment and I would go even beyond that : equals and compareTo have two different business contracts.

        The business contract of equals is to say whether or not two objects represent the same business object. They could be different types (this is implementation) : (Long) 45 and BigInteger(45) are different types but they do represent the same business value. This is the responsibility of the equals method implementation to support or not incompatible types (for a compiler point of view) but the same notion (on the business point of view). Code is not provided to solve compiler issues but to solve business issues. It is better if both types implement the method the same way in order to get a symetric behavior but because both classes may not be developped by the same team this is not always possible. In the end, the question is "Should the class I am developing be compatible with Long and BigInteger because it does represent an integer ?"

        Now, comparing object is another business contract which has nothing to do with identifying whether or not two objects are representing the same business object. Let us go back to maths : equality is the only equivalence relation which all cells have a single element (http://en.wikipedia.org/wiki/Equivalence_relation). On the other hand, an order relation (http://en.wikipedia.org/wiki/Order_relation) induces an equivalence relation (as a < b and b < a means a = b) but this equivalence relation may not be the "equality" equivalence relation because the partition it induces may have more than one object per cells.

        Here is an example : let us consider the order relation "X is older than Y", for Brian and James who are twins we have (Brian < James) and (James < Brian) but of course (Brian = James) is wrong !

        The compareTo method is the definition of such an order on one hand whereas the equals method is responsible to say whether or not two objects represent the same business object. On top of that comes typing... which is 100% low level implementation and 0% business notion : should I need to identify the types of two objects to know if they reprensent the same thing - this is the very questionable Java point of view - ?
        Types are means to help differencing things that have nothing to do with each others but not to separate things that represent the same business object, we are slowly sliding from the technical implementation to what the code is actually manipulating. Between two extrems we have, on one hand with SmallTalk for which, as far as I know, any object can be used to call a method as long as its class implements it and on the other hand Java that refuses to consider that (Long) 50 and new BigInteger(50) are the same thing, there should be a better middle path that could be used by Groovy, especially because Groovy allows operator overloading.

        So, I do think that always using the equals implementation of the L-Value for == should be the rule. The compareTo method has nothing to do with the business contract of == and has everything to do with <=> which is another equivalence relation definition.And this is a very good point for Groovy to differenciate both so why still being partially stuck on the Java point of view which is not coherent and very ambiguous here when a nice and elegant solution is provided by Groovy (the <==> operator) ?
        Both == and <==> could be the same - this would be ideal - and they should be compatible which means (a == b implies a <==> b), but not necessarily (a <==> b implies a == b).

        Show
        Bertrand Francizod added a comment - - edited Peter Rietzler added a comment - 23/Feb/09 3:02 AM let's ignore my use case for the moment - I still stick to my opinion that this issue is a bug and not an improvement. Java defines equality through equals. It is only recommended that Comparable provides the same equality logic, but not required. Therefore == should use equals and not compareTo. Maybe this issue should be set as duplicate to # GROOVY-2334 since the resolution of # GROOVY-2334 would resolve this issue too. --------- I fully agree with this comment and I would go even beyond that : equals and compareTo have two different business contracts. The business contract of equals is to say whether or not two objects represent the same business object. They could be different types (this is implementation) : (Long) 45 and BigInteger(45) are different types but they do represent the same business value. This is the responsibility of the equals method implementation to support or not incompatible types (for a compiler point of view) but the same notion (on the business point of view). Code is not provided to solve compiler issues but to solve business issues. It is better if both types implement the method the same way in order to get a symetric behavior but because both classes may not be developped by the same team this is not always possible. In the end, the question is "Should the class I am developing be compatible with Long and BigInteger because it does represent an integer ?" Now, comparing object is another business contract which has nothing to do with identifying whether or not two objects are representing the same business object. Let us go back to maths : equality is the only equivalence relation which all cells have a single element ( http://en.wikipedia.org/wiki/Equivalence_relation ). On the other hand, an order relation ( http://en.wikipedia.org/wiki/Order_relation ) induces an equivalence relation (as a < b and b < a means a = b) but this equivalence relation may not be the "equality" equivalence relation because the partition it induces may have more than one object per cells. Here is an example : let us consider the order relation "X is older than Y", for Brian and James who are twins we have (Brian < James) and (James < Brian) but of course (Brian = James) is wrong ! The compareTo method is the definition of such an order on one hand whereas the equals method is responsible to say whether or not two objects represent the same business object. On top of that comes typing... which is 100% low level implementation and 0% business notion : should I need to identify the types of two objects to know if they reprensent the same thing - this is the very questionable Java point of view - ? Types are means to help differencing things that have nothing to do with each others but not to separate things that represent the same business object, we are slowly sliding from the technical implementation to what the code is actually manipulating. Between two extrems we have, on one hand with SmallTalk for which, as far as I know, any object can be used to call a method as long as its class implements it and on the other hand Java that refuses to consider that (Long) 50 and new BigInteger(50) are the same thing, there should be a better middle path that could be used by Groovy, especially because Groovy allows operator overloading. So, I do think that always using the equals implementation of the L-Value for == should be the rule. The compareTo method has nothing to do with the business contract of == and has everything to do with <=> which is another equivalence relation definition.And this is a very good point for Groovy to differenciate both so why still being partially stuck on the Java point of view which is not coherent and very ambiguous here when a nice and elegant solution is provided by Groovy (the <==> operator) ? Both == and <==> could be the same - this would be ideal - and they should be compatible which means (a == b implies a <==> b), but not necessarily (a <==> b implies a == b).
        Hide
        blackdrag blackdrag added a comment -

        So basically you suggest a new operator <==>, so that we can write 5.5G<==>5.50Gl to get true and to let 5.5G==5.50G return false.

        Show
        blackdrag blackdrag added a comment - So basically you suggest a new operator <==>, so that we can write 5.5G<==>5.50Gl to get true and to let 5.5G==5.50G return false.
        Hide
        Bertrand Francizod added a comment - - edited

        I am not sure I understand your question. I am not suggesting anything new : the X.equals(Y) method would always correspond to the X == Y operator (this would be the standard mechanism to override this operator for the class that provides the equals method when it is used on the left of the operator) and the X.compareTo(Y) == 0 expression would always correspond to the X <=> Y operator (like in http://docs.codehaus.org/display/GROOVY/Operator+Overloading ), unless I misunderstand the <=> operator as I am a very beginner in Groovy. Both would have distinct and different meanings. What do you think ?

        EDIT : sorry for the typo, I had "<=>" in mind not "<==>".

        Show
        Bertrand Francizod added a comment - - edited I am not sure I understand your question. I am not suggesting anything new : the X.equals(Y) method would always correspond to the X == Y operator (this would be the standard mechanism to override this operator for the class that provides the equals method when it is used on the left of the operator) and the X.compareTo(Y) == 0 expression would always correspond to the X <=> Y operator (like in http://docs.codehaus.org/display/GROOVY/Operator+Overloading ), unless I misunderstand the <=> operator as I am a very beginner in Groovy. Both would have distinct and different meanings. What do you think ? EDIT : sorry for the typo, I had "<=>" in mind not "<==>".
        Hide
        Bertrand Francizod added a comment - - edited

        If you want to override the == operator for existing classes (for example Long and BigInteger) then another mechanism would be required. But, honestly, I am not a great fan of such things because it would be highly misleading, not very readable and a nightmare to debug (C++ish ...). This would also raise security and performance problems when the classes are final.

        Show
        Bertrand Francizod added a comment - - edited If you want to override the == operator for existing classes (for example Long and BigInteger) then another mechanism would be required. But, honestly, I am not a great fan of such things because it would be highly misleading, not very readable and a nightmare to debug (C++ish ...). This would also raise security and performance problems when the classes are final.
        Hide
        Bertrand Francizod added a comment - - edited

        We still have one issue : if == is overriden by the equals method, it is not possible anymore to compare Groovy instances (like == is the means to identify that two references are pointing the same Java object). Maybe another operator would be required, possibly the "<==>" created by my typo or anything else you can imagine.

        Show
        Bertrand Francizod added a comment - - edited We still have one issue : if == is overriden by the equals method, it is not possible anymore to compare Groovy instances (like == is the means to identify that two references are pointing the same Java object). Maybe another operator would be required, possibly the "<==>" created by my typo or anything else you can imagine.
        Hide
        blackdrag blackdrag added a comment -

        For comparing if two objects are referential equals, we have the method "is". <=> is a shortcut for compareTo, so it is supposed to return an int with the values -1,0 or +1 and not a boolean. And we are not done by simply calling compareTo, we normally want to be able to do things like 5==5l, or due to your suggestions 5<=>5l. But this is a type mix, and neither equals nor compareTo usually handle such cases. How do you suggest to solve that?

        Show
        blackdrag blackdrag added a comment - For comparing if two objects are referential equals, we have the method "is". <=> is a shortcut for compareTo, so it is supposed to return an int with the values -1,0 or +1 and not a boolean. And we are not done by simply calling compareTo, we normally want to be able to do things like 5==5l, or due to your suggestions 5<=>5l. But this is a type mix, and neither equals nor compareTo usually handle such cases. How do you suggest to solve that?
        Hide
        Bertrand Francizod added a comment - - edited

        I think we do not understand each others. Maybe the first step would be to clarify what exactly mean in Groovy the == and <=> operators.

        First, in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance. Is this exact mean (or functional contract) preserved in Groovy ? I do not think so because this contract is not compatible with overriding the == operator by calling implicitly the equals method of the object on left of the operator as stated in http://docs.codehaus.org/display/GROOVY/Operator+Overloading . So, please clarify the exact mechanism of the == operator in Groovy.

        Second, if I understand well the http://docs.codehaus.org/display/GROOVY/Operator+Overloading page, "X <=> Y" is inlined by the Groovy compiler by "X.compareTo(Y) == 0" (I do know that X.compareTo(Y) returns an integer).

        EDIT : ok, my bad, I understand now, that "X <=> Y is "X.compareTo(Y)" and not "X.compareTo(Y) == 0". This sounds very wierd to me ! :/

        In the table of the page http://docs.codehaus.org/display/GROOVY/Operator+Overloading, "a == b" is associted with the "a.equals(b) or a.compareTo(b) == 0 ** " expression and the following note :

          • Note: The == operator doesn't always exactly match the .equals() method. You can think of them as equivalent in most situations. In situations where two objects might be thought "equal" via normal Groovy "coercion" mechanisms, the == operator will report them as equal; the .equals() method will not do so if doing so would break the normal rules Java has around the equals method. Expect further improvements to Groovy over time to provide clearer, more powerful and more consistent behavior in this area.

        Sorry, this note does upset me, I do not understand it : de facto, the Java functionnal contract of the == operator is not compatible with the Java functionnal contract of the equals method. So what is stated in this page is a lost battle. It cannot be.
        As stated in my initial post, the compareTo method defines an order relation (pure mathematic definition) which induces a corresponding equivalence relation (defined by the "X.compareTo(Y) == 0" condition). This has nothing to do and is definitely not compatible with the "equality" equivalence relation (again a math concept) defined by the == operator. Maths are here to tell you that it is not possible to mix the compareTo method and the equals method to build any kind of operator definition just because they do not mean the same thing (they do not have the same business contract) AND they are implemented independently. Any attempt to mix both in order to define an operator will fail and result in incoherent behavior because both method implementation can be incompatible. The problem is not at the implementation level, the problem is at the conceptual and design level.

        Maybe this issue deserves an article or a clarification by the Groovy designer team. What do you think ? It is impossible to solve an issue when the problem is not clearly defined.

        Show
        Bertrand Francizod added a comment - - edited I think we do not understand each others. Maybe the first step would be to clarify what exactly mean in Groovy the == and <=> operators. First, in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance. Is this exact mean (or functional contract) preserved in Groovy ? I do not think so because this contract is not compatible with overriding the == operator by calling implicitly the equals method of the object on left of the operator as stated in http://docs.codehaus.org/display/GROOVY/Operator+Overloading . So, please clarify the exact mechanism of the == operator in Groovy. Second, if I understand well the http://docs.codehaus.org/display/GROOVY/Operator+Overloading page, "X <=> Y" is inlined by the Groovy compiler by "X.compareTo(Y) == 0" (I do know that X.compareTo(Y) returns an integer). EDIT : ok, my bad, I understand now, that "X <=> Y is "X.compareTo(Y)" and not "X.compareTo(Y) == 0". This sounds very wierd to me ! :/ In the table of the page http://docs.codehaus.org/display/GROOVY/Operator+Overloading , "a == b" is associted with the "a.equals(b) or a.compareTo(b) == 0 ** " expression and the following note : Note: The == operator doesn't always exactly match the .equals() method. You can think of them as equivalent in most situations. In situations where two objects might be thought "equal" via normal Groovy "coercion" mechanisms, the == operator will report them as equal; the .equals() method will not do so if doing so would break the normal rules Java has around the equals method. Expect further improvements to Groovy over time to provide clearer, more powerful and more consistent behavior in this area. Sorry, this note does upset me, I do not understand it : de facto, the Java functionnal contract of the == operator is not compatible with the Java functionnal contract of the equals method. So what is stated in this page is a lost battle. It cannot be. As stated in my initial post, the compareTo method defines an order relation (pure mathematic definition) which induces a corresponding equivalence relation (defined by the "X.compareTo(Y) == 0" condition). This has nothing to do and is definitely not compatible with the "equality" equivalence relation (again a math concept) defined by the == operator. Maths are here to tell you that it is not possible to mix the compareTo method and the equals method to build any kind of operator definition just because they do not mean the same thing (they do not have the same business contract) AND they are implemented independently. Any attempt to mix both in order to define an operator will fail and result in incoherent behavior because both method implementation can be incompatible. The problem is not at the implementation level, the problem is at the conceptual and design level. Maybe this issue deserves an article or a clarification by the Groovy designer team. What do you think ? It is impossible to solve an issue when the problem is not clearly defined.
        Hide
        Bertrand Francizod added a comment - - edited

        "But this is a type mix, and neither equals nor compareTo usually handle such cases"


        I disagree with your statement because the signature of the equals method defined in the java.lang.Object requires an Object instance and nothing more specific (see http://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#equals-java.lang.Object- ) The reason for this is binary backward compatibility with pre Java 5 compiled code (erasure) . Actually the description of the business contract in the Javadoc sounds incoherent to me also : the equals method cannot have the same result of the == operator or what would be the benefit of it ? The purpose of the method is not to tell if references are pointing the same Java instance but if two different instances are actually representing the same conceptual object ("foo" == "fo"+'o') is wrong but "foo".equals("fo" + 'o') is true.
        Maybe we are just inheriting the ambiguity that was already there in Java, are not we ?

        Show
        Bertrand Francizod added a comment - - edited "But this is a type mix, and neither equals nor compareTo usually handle such cases" I disagree with your statement because the signature of the equals method defined in the java.lang.Object requires an Object instance and nothing more specific (see http://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#equals-java.lang.Object- ) The reason for this is binary backward compatibility with pre Java 5 compiled code (erasure) . Actually the description of the business contract in the Javadoc sounds incoherent to me also : the equals method cannot have the same result of the == operator or what would be the benefit of it ? The purpose of the method is not to tell if references are pointing the same Java instance but if two different instances are actually representing the same conceptual object ("foo" == "fo"+'o') is wrong but "foo".equals("fo" + 'o') is true. Maybe we are just inheriting the ambiguity that was already there in Java, are not we ?
        Hide
        Bertrand Francizod added a comment -

        I am still thinking about all of that... I have the deep feeling that concepts that have nothing to do together are mixed together (maybe the mix is in my mind ! ).
        I am sure of one thing : the equals method and the Comparable interface are totaly incompatible in Java and cannot be mixed to define any operator.

        Show
        Bertrand Francizod added a comment - I am still thinking about all of that... I have the deep feeling that concepts that have nothing to do together are mixed together (maybe the mix is in my mind ! ). I am sure of one thing : the equals method and the Comparable interface are totaly incompatible in Java and cannot be mixed to define any operator.
        Hide
        Paul King added a comment - - edited

        Just to make sure we are on the same page, I'll also have a go at clarifying things (but I'll slightly over-simplify things in a few spots - hopefully not too much). In Groovy, this code "a.is(b)" is translated into bytecode similar to what Java would produce for "a == b". And this code in Groovy "a == b" is translated into bytecode similar to what Java would produce for "a == b" when dealing with primitives and for "a.equals(b)" when dealing with Objects. So Groovy and Java have different syntax in this regard. You need to have a Groovy (not Java) mindset when interpreting the semantics for those operators. The contract for equals and pointer comparison as defined by Java doesn't change, just the syntax used to represent them. Groovy makes the more common goal (business equality) have the more compact representation (syntax). It also unifies the treatment of primitives and Objects for that symbol (i.e. always business equality). Groovy also tries to add a human number/string/collection equality layer on top (which happens to use compareTo) so that e.g. numbers that humans would consider equal (0, 0L, 0.0f, etc.) are also deemed equal but in some sense this is a red herring to understanding "is" and "==". This issue is about an unintended consequence of how Groovy does the number-aware equality comparisons using compareTo. You might also want to look into the canEqual support within @EqualsAndHashCode.

        Show
        Paul King added a comment - - edited Just to make sure we are on the same page, I'll also have a go at clarifying things (but I'll slightly over-simplify things in a few spots - hopefully not too much). In Groovy, this code " a.is(b) " is translated into bytecode similar to what Java would produce for " a == b ". And this code in Groovy " a == b " is translated into bytecode similar to what Java would produce for " a == b " when dealing with primitives and for " a.equals(b) " when dealing with Objects. So Groovy and Java have different syntax in this regard. You need to have a Groovy (not Java) mindset when interpreting the semantics for those operators. The contract for equals and pointer comparison as defined by Java doesn't change, just the syntax used to represent them. Groovy makes the more common goal (business equality) have the more compact representation (syntax). It also unifies the treatment of primitives and Objects for that symbol (i.e. always business equality). Groovy also tries to add a human number/string/collection equality layer on top (which happens to use compareTo) so that e.g. numbers that humans would consider equal (0, 0L, 0.0f, etc.) are also deemed equal but in some sense this is a red herring to understanding "is" and "==". This issue is about an unintended consequence of how Groovy does the number-aware equality comparisons using compareTo. You might also want to look into the canEqual support within @EqualsAndHashCode .
        Hide
        blackdrag blackdrag added a comment -

        "in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance."
        This is not entirely true, it is not the native types, it is the primitive types. so int==long can be done, but Integer==Long will always yield false. For this primitive type mechanism Java has Widening Primitive Conversion (http://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html#jls-5.1.2)

        Groovy does not follow these exact semantics. Traditionally the number 1 is in Groovy an Integer and not an int. Thus we traditionally do not have primitives at all, only native types, which would be for the integral types int, long, short, byte and BigInteger. Between those we allow conversions in terms of widening and narrowing (byte = int is allowed without cast in Groovy). So unlike what that page may suggest to you, we can neither simply call equals, not can we simply call compareTo, or something like 5==5l would be always false. Thus we have some conversion code depending on the involved types we decide if we call equals or compareTo with or without conversions. In case of 5=5l we will traditionally call https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/LongMath.java#L49 thus it will promote the Integer to a long and then not call compareTo or equals at all.

        Since Groovy 1.8 groovy supports also primitives that stay primitives (before they have been boxed on the first possible chance), so in the case of 5=5l we will do the widening in bytecode and use bytecode instructions to compare the primitive longs instead of going through NumberMath. But if you have a==b and a is an Integer 5 and b a Long 5l, then we still go through that path.

        So unlike Java we have widening not only for primitive types, when we do ==. We have several conversions/actions special for GString/String, our number types, lists and maps. Only if none of these apply we call compareTo for Comparables and equals as last resort. Referential identity not used for == by us at all, unless an equals method we call does that.

        I would say that makes the Groovy logic for == similar in many aspects to Java, but also very different.

        Is for <=>, this is also not purely calling compareTo. Both == and <=> go through https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java#L533 the only difference being that equalityCheckOnly is true for == and false for <=>. So we have 5<=>5l return 0 for example. Part of the problem is that compareTo is not required to be defined for any value, like equals. So compareTo is allowed to throw exceptions. Thus, if we have a<=>b, and b is not an instance of the class of a, then we don't call compareTo either and instead produce a GroovyRuntimeException.

        What http://docs.codehaus.org/display/GROOVY/Operator+Overloading is a simplification, maybe an oversimplification in some places.

        Defining the problem for == is not easy, but let me try. Basically we want to make == sense for all primitives, for maps, lists, String, GString, Character and all normal Numbers, which are Short, Byte, Integer, Long, BigInteger, Float, Double and BigDecimal and to some extend the mix of these types. And that in the sense of a somehow equal value. Neither equals, nor compareTo can do that.

        That compareTo is called for an type not included in the set above is surely a point we can discuss. But we cannot ignore the cases above.

        Show
        blackdrag blackdrag added a comment - "in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance." This is not entirely true, it is not the native types, it is the primitive types. so int==long can be done, but Integer==Long will always yield false. For this primitive type mechanism Java has Widening Primitive Conversion ( http://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html#jls-5.1.2 ) Groovy does not follow these exact semantics. Traditionally the number 1 is in Groovy an Integer and not an int. Thus we traditionally do not have primitives at all, only native types, which would be for the integral types int, long, short, byte and BigInteger. Between those we allow conversions in terms of widening and narrowing (byte = int is allowed without cast in Groovy). So unlike what that page may suggest to you, we can neither simply call equals, not can we simply call compareTo, or something like 5==5l would be always false. Thus we have some conversion code depending on the involved types we decide if we call equals or compareTo with or without conversions. In case of 5=5l we will traditionally call https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/LongMath.java#L49 thus it will promote the Integer to a long and then not call compareTo or equals at all. Since Groovy 1.8 groovy supports also primitives that stay primitives (before they have been boxed on the first possible chance), so in the case of 5=5l we will do the widening in bytecode and use bytecode instructions to compare the primitive longs instead of going through NumberMath. But if you have a==b and a is an Integer 5 and b a Long 5l, then we still go through that path. So unlike Java we have widening not only for primitive types, when we do ==. We have several conversions/actions special for GString/String, our number types, lists and maps. Only if none of these apply we call compareTo for Comparables and equals as last resort. Referential identity not used for == by us at all, unless an equals method we call does that. I would say that makes the Groovy logic for == similar in many aspects to Java, but also very different. Is for <=>, this is also not purely calling compareTo. Both == and <=> go through https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java#L533 the only difference being that equalityCheckOnly is true for == and false for <=>. So we have 5<=>5l return 0 for example. Part of the problem is that compareTo is not required to be defined for any value, like equals. So compareTo is allowed to throw exceptions. Thus, if we have a<=>b, and b is not an instance of the class of a, then we don't call compareTo either and instead produce a GroovyRuntimeException. What http://docs.codehaus.org/display/GROOVY/Operator+Overloading is a simplification, maybe an oversimplification in some places. Defining the problem for == is not easy, but let me try. Basically we want to make == sense for all primitives, for maps, lists, String, GString, Character and all normal Numbers, which are Short, Byte, Integer, Long, BigInteger, Float, Double and BigDecimal and to some extend the mix of these types. And that in the sense of a somehow equal value. Neither equals, nor compareTo can do that. That compareTo is called for an type not included in the set above is surely a point we can discuss. But we cannot ignore the cases above.
        Hide
        Bertrand Francizod added a comment -

        Thanks a lot to both of you for your answers. They are giving me a new light on the problem and I will add elements to this thread as soon as I have digested all of this.

        Show
        Bertrand Francizod added a comment - Thanks a lot to both of you for your answers. They are giving me a new light on the problem and I will add elements to this thread as soon as I have digested all of this.
        Hide
        Patrick Ryan added a comment -

        You have made a cogent argument around the complexities involved - but the bottom line in all of this is that the following does not work in a DSL:

        asset x == 1

        I have a DSL like:

        if( responseTo('MyHeight') == 5 ) .....

        where responseTo returns a complex object which includes the users selected units from the authoring environment, along with other pieces of information.

        For my response object, I was expecting/hoping that either .equals() or .compareTo() would handle this situation.

        Since I cannot change the DSL, because I am trying to create a script to execute the existing DSL, I am in the position of trying to figure out how to support this kind of syntax.

        Given this is my current situation - does anyone have a suggestion on how I could support all of the logical operators for the above DSL snippet?

        Thank you

        Show
        Patrick Ryan added a comment - You have made a cogent argument around the complexities involved - but the bottom line in all of this is that the following does not work in a DSL: asset x == 1 I have a DSL like: if( responseTo('MyHeight') == 5 ) ..... where responseTo returns a complex object which includes the users selected units from the authoring environment, along with other pieces of information. For my response object, I was expecting/hoping that either .equals() or .compareTo() would handle this situation. Since I cannot change the DSL, because I am trying to create a script to execute the existing DSL, I am in the position of trying to figure out how to support this kind of syntax. Given this is my current situation - does anyone have a suggestion on how I could support all of the logical operators for the above DSL snippet? Thank you
        Hide
        blackdrag blackdrag added a comment -

        You know, you can always use an AST transformation and change the AST to use different methods.

        Show
        blackdrag blackdrag added a comment - You know, you can always use an AST transformation and change the AST to use different methods.

          People

          • Assignee:
            Unassigned
            Reporter:
            Peter Rietzler
          • Votes:
            15 Vote for this issue
            Watchers:
            14 Start watching this issue

            Dates

            • Created:
              Updated: