groovy

null's in lists are not handled correctly for unique(), sort()

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.1-beta-1
  • Fix Version/s: 1.1-beta-2
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    0

Description

For this list:

def x = [1, 2, 3, 1, 2, 3, null, 'a', null]

Both these throw NPE:

println x.unique()
println x.sort()

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Michael Baehr added a comment -

In DefaultGroovyMethods.java, locate the inner class NumberComparator.

Instead of
public int compare(Object o1, Object o2) {
if (o1 instanceof Number && o2 instanceof Number) {
...

check for null first:

public int compare(Object o1, Object o2) {
if (o1 == null) { return o2==null?0:-1; } else if (o2 == null) { return 1; } else if (o1 instanceof Number && o2 instanceof Number) {
...

This solves the NPE problem.

Show
Michael Baehr added a comment - In DefaultGroovyMethods.java, locate the inner class NumberComparator. Instead of public int compare(Object o1, Object o2) { if (o1 instanceof Number && o2 instanceof Number) { ... check for null first: public int compare(Object o1, Object o2) { if (o1 == null) { return o2==null?0:-1; } else if (o2 == null) { return 1; } else if (o1 instanceof Number && o2 instanceof Number) { ... This solves the NPE problem.
Hide
Permalink
Michael Baehr added a comment -

Just discovered the code tag ...

In DefaultGroovyMethods.java, locate the inner class NumberComparator.

Instead of

public int compare(Object o1, Object o2) {
  if (o1 instanceof Number && o2 instanceof Number) {
    ...

check for null first:

public int compare(Object o1, Object o2) {
  if (o1 == null) { 
    return o2==null?0:-1; 
  } else if (o2 == null) {
    return 1; 
  } else if (o1 instanceof Number && o2 instanceof Number) {
    ...

This solves the NPE problem.

Show
Michael Baehr added a comment - Just discovered the code tag ... In DefaultGroovyMethods.java, locate the inner class NumberComparator. Instead of 
public int compare(Object o1, Object o2) {
  if (o1 instanceof Number && o2 instanceof Number) {
    ...
check for null first: 
public int compare(Object o1, Object o2) {
  if (o1 == null) { 
    return o2==null?0:-1; 
  } else if (o2 == null) {
    return 1; 
  } else if (o1 instanceof Number && o2 instanceof Number) {
    ...
This solves the NPE problem.
Hide
Permalink
Paul King added a comment -

Patch and tests added.

Show
Paul King added a comment - Patch and tests added.
Hide
Permalink
Bob Samuels added a comment -

I believe this is broken again. I had a list that held objects whose properties (which I'm sorting on) could be null and received a NPE. ie->
class test{ String name Integer sortValue }

My collection would contain 10 test objects, some with a sortValue, some without. When I sort by this value, I get an NPE at the first null value. Shouldn't the nulls float to the end?

Show
Bob Samuels added a comment - I believe this is broken again. I had a list that held objects whose properties (which I'm sorting on) could be null and received a NPE. ie-> class test{ String name Integer sortValue } My collection would contain 10 test objects, some with a sortValue, some without. When I sort by this value, I get an NPE at the first null value. Shouldn't the nulls float to the end?
Hide
Permalink
blackdrag blackdrag added a comment -

null is the smallest value, so having it at the beginning is ok I think

Show
blackdrag blackdrag added a comment - null is the smallest value, so having it at the beginning is ok I think
Hide
Permalink
Paul King added a comment -

There is a little bit more we could do to get rid of a NPE here. I'll investigate.

Show
Paul King added a comment - There is a little bit more we could do to get rid of a NPE here. I'll investigate.
Hide
Permalink
Paul King added a comment -

An additional edge case is now covered and will appear in 1.7.3.

Show
Paul King added a comment - An additional edge case is now covered and will appear in 1.7.3.
Hide
Permalink
Paul King added a comment -

Workaround in the meantime is to use the spaceship operator version, i.e. instead of:

list.sort{ it?.sortValue }

use:

list.sort{ a, b -> a?.sortValue <=> b?.sortValue }
Show
Paul King added a comment - Workaround in the meantime is to use the spaceship operator version, i.e. instead of: 
list.sort{ it?.sortValue }
use: 
list.sort{ a, b -> a?.sortValue <=> b?.sortValue }

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.