Uploaded image for project: 'Commons BeanUtils'
  1. Commons BeanUtils
  2. BEANUTILS-53

BeanComparator should handle null property values (?)

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • Nightly Builds
    • None
    • None
    • None
    • Operating System: other
      Platform: Other

    • 18791

    Description

      Assume a "Person" with 3 string properties age, name, hairColor. We've got
      three people, Bob, Jane, and John - John happens to be bald, and his hairColor
      property is "null". Trying to sort a collection of Person beans by hairColor
      with a null entry throws a ClassCastException from ComparableComparator.

      " java.lang.ClassCastException: There were nulls in the arguments for this
      method: compare(hawkeye, null)' "

      One should be able to sort a collection of beans on a property which contains
      null values. The question that remains is whether the comparing with a null
      should return a 1 or a -1 (do you want your nulls in the beginning or at the
      end?). I'd assume at the end as the default behaviour - maybe add a property
      to allow people to customize this.

      Attachments

        Activity

          When I originally wrote the BeanComparator, I had that type of functionality,
          but it was removed for the initial commit because of exactly that issue. Who
          comes first, the null or the not null value?

          To get around this, I wrote a NullComparator that allows you to specify who
          comes first, the null or the not null value. I then wrapped the bean
          comparator in the null comparator using the comparatorChain.

          I think add this functionality directly confuses the BeanComparator. And also,
          you could argue other comparators suffer from the same issue with nulls. If
          you want to see some code, let me know.
          Eric

          dep4b David Eric Pugh added a comment - When I originally wrote the BeanComparator, I had that type of functionality, but it was removed for the initial commit because of exactly that issue. Who comes first, the null or the not null value? To get around this, I wrote a NullComparator that allows you to specify who comes first, the null or the not null value. I then wrapped the bean comparator in the null comparator using the comparatorChain. I think add this functionality directly confuses the BeanComparator. And also, you could argue other comparators suffer from the same issue with nulls. If you want to see some code, let me know. Eric
          mohankishore@yahoo.com Mohan Kishore added a comment -

          There's a class org.apache.commons.collections.comparators.NullComparator,
          which can wrap around any other comparator. It also allows you to specify
          whether the nulls come out on the top or bottom.

          would it make sense to promote the class to 'lang' and use it?

          mohankishore@yahoo.com Mohan Kishore added a comment - There's a class org.apache.commons.collections.comparators.NullComparator, which can wrap around any other comparator. It also allows you to specify whether the nulls come out on the top or bottom. would it make sense to promote the class to 'lang' and use it?

          Not sure what you mean by promote it to "lang", I think it definitly belongs
          where it is. However, if you mean promote it to commons, I believe it already
          is. HOwever, maybe some javadocs might be nice for people to use?

          Or do you mean not adding a dependency on commons-collections onto beanutils?
          Eric

          dep4b David Eric Pugh added a comment - Not sure what you mean by promote it to "lang", I think it definitly belongs where it is. However, if you mean promote it to commons, I believe it already is. HOwever, maybe some javadocs might be nice for people to use? Or do you mean not adding a dependency on commons-collections onto beanutils? Eric
          mohankishore@yahoo.com Mohan Kishore added a comment -

          I was talking about the dependencies concern.

          As such a comparator seems more in place in bean utils, rather than collections
          module. If more modules might use it, then it might be better placed in lang
          module - imho. just a suggestion, do not claim enough understanding of the
          division of labour to make a stronger recommendation...

          mohankishore@yahoo.com Mohan Kishore added a comment - I was talking about the dependencies concern. As such a comparator seems more in place in bean utils, rather than collections module. If more modules might use it, then it might be better placed in lang module - imho. just a suggestion, do not claim enough understanding of the division of labour to make a stronger recommendation...

          I am somewhat in the same bag.. There is a real tendency to put everything in
          lang! Pretty much ANY code I write ends up having a dependency on beanutils,
          collections, and lang... Almost as if there should be commons-core!

          ERic

          dep4b David Eric Pugh added a comment - I am somewhat in the same bag.. There is a real tendency to put everything in lang! Pretty much ANY code I write ends up having a dependency on beanutils, collections, and lang... Almost as if there should be commons-core! ERic
          tobrien@discursive.com Tim O'Brien added a comment -

          Created an attachment (id=5714)
          patch for BeanComparator javadoc

          tobrien@discursive.com Tim O'Brien added a comment - Created an attachment (id=5714) patch for BeanComparator javadoc
          tobrien@discursive.com Tim O'Brien added a comment -

          There's a patch for the Javadoc, it adds a hint for those who are in my
          situation. I agree that it would confuse BeanComparator to add a
          NullComparator by default, this patch adds a note for people who are in my
          situation - think of it as a hint.

          Let me know if the patch is kosher, and I'll commit it.

          tobrien@discursive.com Tim O'Brien added a comment - There's a patch for the Javadoc, it adds a hint for those who are in my situation. I agree that it would confuse BeanComparator to add a NullComparator by default, this patch adds a note for people who are in my situation - think of it as a hint. Let me know if the patch is kosher, and I'll commit it.
          rdonkin@apache.org rdonkin@apache.org added a comment -

          I've committed tim's javadoc improvements

          rdonkin@apache.org rdonkin@apache.org added a comment - I've committed tim's javadoc improvements

          People

            Unassigned Unassigned
            tobrien@discursive.com Tim O'Brien
            Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: