0

I have a misunderstanding about Comparator interface and its method compare here is the following code and i am wondering why compare method return -33 i believe that it should return 33

import java.util.*;
public class Sorted implements Comparable<Sorted>, Comparator<Sorted> {
private int num;
private String text;

Sorted(int n, String t) {
    this.num = n;
    this.text = t;
}

public String toString() {
    return "" + num;
}

public int compareTo(Sorted s) {
    return text.compareTo(s.text);
}

public int compare(Sorted s1, Sorted s2) {
    System.out.println(s1.num-s2.num); // return -33
    return s1.num - s2.num;
}
public static void main(String[] args) {
    Sorted s1 = new Sorted(88, "a");
    Sorted s2 = new Sorted(55, "b");
    TreeSet<Sorted> t1 = new TreeSet<>();
    t1.add(s1); t1.add(s2);
    TreeSet<Sorted> t2 = new TreeSet<>(s1);
    t2.add(s1); t2.add(s2);
    System.out.println(t1 + " " + t2);
    System.out.println(s1.num-s2.num); // prints 33
} }
user2780962
  • 113
  • 2
  • 3
  • 10
  • The s1 and s2 in compare(Sorted s1, Sorted s2) are local variable definitions, you must not confuse them with the definitions in main(). It is not defined (algorithmically, only by the implementation) how TreeSet compares the two elements. – leonardkraemer Feb 16 '19 at 10:38
  • sure, i agree with you. but this is what i am trying to figure out (how TreeSet compares the tow element) – user2780962 Feb 16 '19 at 10:42

2 Answers2

4

You probably know that if a-b=c then b-a=-c.

What happens here is very similar. You seem to have assumed that TreeSet is calls the compare method like this:

comparator.compare(s1, s2)

(Note that I used s1 and s2 for demonstrative purposes. They are obviously not in scope in TreeSet. s1 is the same instance as your s1 and s2 is the same instance as your s2`.)

But it could call compare like this as well:

comparator.compare(s2, s1)

couldn't it?

If it called it the second way, then the result of -33 is to be expected.

EDIT:

I looked into the source code for TreeSet.add and found that it calls TreeMap.put with the item you are adding as the key. If you look further into TreeMap.put, you will find:

Comparator<? super K> cpr = comparator;
if (cpr != null) {
    do {
        parent = t;
        cmp = cpr.compare(key, t.key); // <--- "key" is the key passed into this method
                                       // "t" is an element that is already in the map
        if (cmp < 0)
            t = t.left;
        else if (cmp > 0)
            t = t.right;
        else
            return t.setValue(value);
    } while (t != null);
}

This shows that TreeSet indeed calls compare the way I described.

EDIT:

As Holger said in the comments, you should not implement Comparator by subtracting the two integers. Instead, you should use Integer.compare:

return Integer.compare(s1.num, s2.num);

In fact, there is no need to implement Comparator at all, you can pass in a Comparator.comparingInt(s -> s.num) when you create the TreeMap:

TreeSet<Sorted> t1 = new TreeSet<>(Comparator.comparingInt(s -> s.num));
Sweeper
  • 213,210
  • 22
  • 193
  • 313
  • i agree with you but in some other programs it calls it the normal way and not viceversa , how do i know how the Treeset calls compare method – user2780962 Feb 16 '19 at 10:40
  • @user2780962 You don't need to know that, unless you are trying to write a `TreeSet` yourself. In the `compare` method, your only job is to compare the two objects. You shouldn't depend on the order of arguments. If you are just curious, I have added some of the source code that shows how `compare` is called. – Sweeper Feb 16 '19 at 10:45
  • 2
    @user2780962 What you need to understand, is that you **can't** rely on any order. You have to provide a good comparator, that's all. – Jean-Baptiste Yunès Feb 16 '19 at 10:46
  • @user2780962 Did my answer answer your question? If so, please consider accepting it by clicking on that checkmark! – Sweeper Feb 16 '19 at 11:13
  • 1
    Besides that, you should not use minus for a comparator anyway. The difference between two `int` values can be bigger than the `int` value range, hence, overflow. As an additional corner case, when the difference is exactly `Integer.MIN_VALUE`, it will be asymmetric. The correct solution would be calling `Integer.compare(s1.num, s2.num)` instead, though, it is a questionable design to let this class implement `Comparator` anyway. Use `new TreeSet<>(Comparator.comparingInt(s -> s.num))` to create a set sorted by `num`. – Holger Feb 18 '19 at 08:29
  • @Holger You are right, but that's not OP's question, is it? That's why I did not include it in the answer. – Sweeper Feb 18 '19 at 08:32
  • I think, it’s worth pointing out these things, in addition to the answer, instead of letting the OP go, thinking that all their problems were solved. – Holger Feb 18 '19 at 08:34
  • @Holger Okay. Edited. – Sweeper Feb 18 '19 at 08:38
0

The s1 and s2 in compare(Sorted s1, Sorted s2) are local variable definitions, you must not confuse them with the definitions in main(). It is not defined (algorithmically, only by the implementation) how TreeSet compares the two elements.

compare(s1, s2) //yields 33
compare(s2, s1) //yields -33

TreeSet internally uses a TreeMap. put calls compare in several places, usually with the element you put into the TreeSet as first element. Therefore put(s2)will call compare(s2, s1). See Code excerpt below:

public V put(K key, V value) {    
        Entry<K,V> t = root;    
        if (t == null) {
            compare(key, key); // type (and possibly null) check       
            root = new Entry<>(key, value, null);  
            size = 1;
            modCount++;    
            return null;    
        }    
        int cmp;    
        Entry<K,V> parent;    
        // split comparator and comparable paths    
        Comparator<? super K> cpr = comparator;    
        if (cpr != null) {    
            do {
                parent = t;    
                cmp = cpr.compare(key, t.key);    
                if (cmp < 0)   
                    t = t.left;    
                else if (cmp > 0)    
                    t = t.right;    
                else    
                    return t.setValue(value);    
            } while (t != null);    
        }    
        else {   
            if (key == null)    
                throw new NullPointerException();    
            @SuppressWarnings("unchecked")    
                Comparable<? super K> k = (Comparable<? super K>) key;   
            do {    
                parent = t;    
                cmp = k.compareTo(t.key);    
                if (cmp < 0)
                     t = t.left; 
                else if (cmp > 0)
                    t = t.right;
                else
                    return t.setValue(value);    
            } while (t != null);    
        }    
        Entry<K,V> e = new Entry<>(key, value, parent);    
        if (cmp < 0)
                parent.left = e;    
        else    
            parent.right = e;    
        fixAfterInsertion(e);    
        size++;    
        modCount++;

        return null;
    }

Other implementations or methods might have different behaviour.

leonardkraemer
  • 6,573
  • 1
  • 31
  • 54