groovy
  1. groovy
  2. GROOVY-5056

?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

        Hide
        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 }
        Hide
        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.
        Hide
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: