Commons Lang
  1. Commons Lang
  2. LANG-667

Add a Null-safe compare() method to ObjectUtils

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 3.0
    • Component/s: lang.*
    • Labels:
      None

      Description

      Add a Null-safe compare() method to ObjectUtils

        Activity

        Hide
        Paul Benedict added a comment -

        "This feature is not a Comparable implementation and so breaks no contract." Agreed. It would be nice to note that in the method's javadoc. Otherwise, I withdraw my objection.

        Show
        Paul Benedict added a comment - "This feature is not a Comparable implementation and so breaks no contract." Agreed. It would be nice to note that in the method's javadoc. Otherwise, I withdraw my objection.
        Hide
        Niall Pemberton added a comment -

        Simplified the code as per Julien's suggestion:

        http://svn.apache.org/viewvc?view=revision&revision=1056520

        Show
        Niall Pemberton added a comment - Simplified the code as per Julien's suggestion: http://svn.apache.org/viewvc?view=revision&revision=1056520
        Hide
        Stephen Colebourne added a comment -

        I support this change, its a useful feature that is clearly documented and no threat to the Comparable contract.

        Show
        Stephen Colebourne added a comment - I support this change, its a useful feature that is clearly documented and no threat to the Comparable contract.
        Hide
        Niall Pemberton added a comment -

        @Julien - thanks for the suggestion, I'll look at this when I get home

        @Paul - Lang is all about adding useful utilities that Java itself doesn't have and theres alot of prior art here of null-safe utilities. Julien's example is a great demonstration of how this function can be used to simplify code. This feature is not a Comparable implementation and so breaks no contract. I really don't understand your objection and IMO its a very weak argument to object on the basis of Comparable's JavaDoc. How would you handle implementing a compareTo() method where the properties being compared may be null - surely you're not arguing that Julien's example should fail and throw a NullPointerException if one of the properties is null?

        Show
        Niall Pemberton added a comment - @Julien - thanks for the suggestion, I'll look at this when I get home @Paul - Lang is all about adding useful utilities that Java itself doesn't have and theres alot of prior art here of null-safe utilities. Julien's example is a great demonstration of how this function can be used to simplify code. This feature is not a Comparable implementation and so breaks no contract. I really don't understand your objection and IMO its a very weak argument to object on the basis of Comparable's JavaDoc. How would you handle implementing a compareTo() method where the properties being compared may be null - surely you're not arguing that Julien's example should fail and throw a NullPointerException if one of the properties is null?
        Hide
        Julien Aymé added a comment -

        Regarding the code, I think it would be faster and simpler to understand if we add a "==" test first:

        public static <T extends Comparable<? super T>> int compare(T c1, T c2, boolean nullGreater) {
            if (c1 == c2) {
                return 0;
            } else if (c1 == null) {
                return nullGreater ? 1 : -1;
            } else if (c2 == null) {
                return nullGreater ? -1 : 1;
            }
            return c1.compareTo(c2);
        }
        

        Note: the commited code in rev 1056124 was:

        public static <T extends Comparable<? super T>> int compare(T c1, T c2, boolean nullGreater) {
            int result = 0;
            if ((c1 == null) || (c2 == null)) {
                if (nullGreater) {
                    result = (c1 == null ? 1 : 0) - (c2 == null ? 1 : 0);
                } else {
                    result = (c1 == null ? -1 : 0) - (c2 == null ? -1 : 0);
                }
            } else {
                result = c1.compareTo(c2);
            }
            return result;
        }
        
        Show
        Julien Aymé added a comment - Regarding the code, I think it would be faster and simpler to understand if we add a "==" test first: public static <T extends Comparable<? super T>> int compare(T c1, T c2, boolean nullGreater) { if (c1 == c2) { return 0; } else if (c1 == null ) { return nullGreater ? 1 : -1; } else if (c2 == null ) { return nullGreater ? -1 : 1; } return c1.compareTo(c2); } Note: the commited code in rev 1056124 was: public static <T extends Comparable<? super T>> int compare(T c1, T c2, boolean nullGreater) { int result = 0; if ((c1 == null ) || (c2 == null )) { if (nullGreater) { result = (c1 == null ? 1 : 0) - (c2 == null ? 1 : 0); } else { result = (c1 == null ? -1 : 0) - (c2 == null ? -1 : 0); } } else { result = c1.compareTo(c2); } return result; }
        Hide
        Julien Aymé added a comment -

        Even is this method does break the contract of Comparable, it is really usefull when comparing many fields of two comparable beans (which may be null):

        public int compareTo(Bean other) {
            int c = ObjectUtils.compare(this.getField1(), other.getField1());
            if (c != 0) {
                return c;
            }
            c = ObjectUtils.compare(this.getField2(), other.getField2());
            if (c != 0) {
                return c;
            }
            return ObjectUtils.compare(this.getField3(), other.getField3());
        }
        

        I don't want to be forced to handle null on field1, 2, or 3 for the 2 beans (and that's why in my projects I already have this method added to ObjectUtils).
        I propose that we add a javadoc comment stating that the method breaks the Comparable contract.

        Show
        Julien Aymé added a comment - Even is this method does break the contract of Comparable, it is really usefull when comparing many fields of two comparable beans (which may be null): public int compareTo(Bean other) { int c = ObjectUtils.compare( this .getField1(), other.getField1()); if (c != 0) { return c; } c = ObjectUtils.compare( this .getField2(), other.getField2()); if (c != 0) { return c; } return ObjectUtils.compare( this .getField3(), other.getField3()); } I don't want to be forced to handle null on field1, 2, or 3 for the 2 beans (and that's why in my projects I already have this method added to ObjectUtils). I propose that we add a javadoc comment stating that the method breaks the Comparable contract.
        Hide
        Paul Benedict added a comment -

        This feature sounds like a bad idea because it goes against the contract of Comparable. From the javadocs:
        "e.compareTo(null) should throw a NullPointerException even though e.equals(null) returns false."

        There is explicitly no such thing as a null-safe compare() in Java.

        Show
        Paul Benedict added a comment - This feature sounds like a bad idea because it goes against the contract of Comparable. From the javadocs: "e.compareTo(null) should throw a NullPointerException even though e.equals(null) returns false." There is explicitly no such thing as a null-safe compare() in Java.
        Show
        Niall Pemberton added a comment - Added: http://svn.apache.org/viewvc?view=revision&revision=1056124

          People

          • Assignee:
            Niall Pemberton
            Reporter:
            Niall Pemberton
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development