groovy

?TreeSet? Comparators not correctly comparing everything - Compares only a subset

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.8.2
  • Fix Version/s: 1.9-beta-4, 1.8.4
  • Component/s: None
  • Labels:
    None
  • Testcase included:
    yes
  • Number of attachments :
    1

Description

Hi everyone

This fails with the latest 1.8.3 snapshot - But works in 1.7.10

import java.util.logging.Logger;
import org.junit.*;

class Breaker {
   static Logger log = Logger.getLogger(getName())
   
   @Test
   void testBreaking() {
      def comparator = [compare:
         {a,b-> 
            def retVal = a.x.compareTo(b.x)
            log.info("Comparing ${a.x} to ${b.x} and returning ${retVal}")
            return retVal }
      ] as Comparator
   
      def ts1 = new TreeSet(comparator)
      ts1.addAll([
         new ToCompare(x:"1"),
         new ToCompare(x:"2"),
         new ToCompare(x:"3")
      ])
       
      def ts2 = new TreeSet(comparator)
      ts2.addAll([
         new ToCompare(x:"1"),
         new ToCompare(x:"2"),
         new ToCompare(x:"3")
      ])
      
      def difference = ts1 - ts2
      assert difference.size() == 0
   }
}

class ToCompare {
   String x
}

The test works if you pass the same list of the same objects to ts1 and ts2. Passing in different objects with the same content fails.

For 1.8.x, the logs spit out:

Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 2 to 1 and returning 1
Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 1 and returning 2
Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 2 and returning 1
Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 2 to 1 and returning 1
Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 1 and returning 2
Sep 29, 2011 9:03:57 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 2 and returning 1

For 1.7.10, the logs spit out:


Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 2 to 1 and returning 1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 1 and returning 2
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 2 and returning 1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 2 to 1 and returning 1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 1 and returning 2
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 2 and returning 1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 1 to 2 and returning -1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 1 to 1 and returning 0
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 2 to 2 and returning 0
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 2 and returning 1
Sep 29, 2011 9:07:09 PM sun.reflect.NativeMethodAccessorImpl invoke0
INFO: Comparing 3 to 3 and returning 0

Please let me know if there's anything else ya'll need!

Cheers
Jason Griffith

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Roshan Dawrani added a comment -

Attaching a patch, that also fixes the following:

@Test
void testBreaking2() {
  def comparator = [compare:
     {a,b-> 
        def retVal = a.x.compareTo(b.x)
        println("Comparing ${a.x} to ${b.x} and returning ${retVal}")
        return retVal }
  ] as Comparator

  def ts1 = new TreeSet(comparator)
  ts1.addAll([
     new ToCompare(x:"1"),
     new ToCompare(x:"2"),
     new ToCompare(x:"3")
  ])

  def difference = ts1 - new ToCompare(x:"3")
  assert difference.size() == 2
}
Show
Roshan Dawrani added a comment - Attaching a patch, that also fixes the following: @Test void testBreaking2() { def comparator = [compare: {a,b-> def retVal = a.x.compareTo(b.x) println( "Comparing ${a.x} to ${b.x} and returning ${retVal}" ) return retVal } ] as Comparator def ts1 = new TreeSet(comparator) ts1.addAll([ new ToCompare(x: "1" ), new ToCompare(x: "2" ), new ToCompare(x: "3" ) ]) def difference = ts1 - new ToCompare(x: "3" ) assert difference.size() == 2 }
Roshan Dawrani made changes -
Field Original Value New Value
Attachment Groovy5056_v1_8_3.diff [ 57089 ]
Hide
Permalink
blackdrag blackdrag added a comment -

Roshan, the fix looks good to me, but testcases are needed.

Show
blackdrag blackdrag added a comment - Roshan, the fix looks good to me, but testcases are needed.
Roshan Dawrani made changes -
Assignee Roshan Dawrani [ roshandawrani ]
Roshan Dawrani made changes -
Status Open [ 1 ] Resolved [ 5 ]
Fix Version/s 1.8.4 [ 17852 ]
Fix Version/s 1.9-beta-4 [ 17656 ]
Resolution Fixed [ 1 ]
Paul King made changes -
Status Resolved [ 5 ] Closed [ 6 ]
Hide
Permalink
Jason Griffith added a comment -

Just tried out the current 1.8.4 snapshot today. All of our tests pass! Thank you all very much for your swift and capable handling of these issues!

Show
Jason Griffith added a comment - Just tried out the current 1.8.4 snapshot today. All of our tests pass! Thank you all very much for your swift and capable handling of these issues!

People

  • Assignee:
    Roshan Dawrani
    Reporter:
    Jason Griffith
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: