10

Suppose I have a class not implementing the Comparable interface like

class Dummy {
}

and a collection of instances of this class plus some function external to the class that allows comparing these instances partially (a map will be used for this purpose below):

Collection<Dummy> col = new ArrayList<>();
Map<Dummy, Integer> map = new HashMap<>();
for (int i = 0; i < 12; i++) {
    Dummy d = new Dummy();
    col.add(d);
    map.put(d, i % 4);
}

Now I want to sort this collection using the TreeSet class with a custom comparator:

TreeSet<Dummy> sorted = new TreeSet<>(new Comparator<Dummy>() {
    @Override
    public int compare(Dummy o1, Dummy o2) {
        return map.get(o1) - map.get(o2);
    }
});
sorted.addAll(col);

The result is obviously unsatisfactory (contains less elements than the initial collection). This is because such a comparator is not consistent with equals, i.e. sometimes returns 0 for non-equal elements. My next attempt was to change the compare method of the comparator to

@Override
public int compare(Dummy o1, Dummy o2) {
    int d = map.get(o1) - map.get(o2);
    if (d != 0)
        return d;
    if (o1.equals(o2))
        return 0;
    return 1; // is this acceptable?
}

It seemingly gives the desired result for this simple demonstrational example but I'm still in doubt: is it correct to always return 1 for unequal (but undistinguishable by the map) objects? Such a relation still violates the general contact for the Comparator.compare() method because sgn(compare(x, y)) == -sgn(compare(y, x)) is, generally, wrong. Do I really need to implement a correct total ordering for TreeSet to work correctly or the above is enough? How to do this when an instance has no fields to compare?

For more real-life example imagine that, instead of Dummy, you have a type parameter T of some generic class. T may have some fields and implement the equals() method through them, but you don't know these fields and yet need to sort instances of this class according to some external function. Is this possible with the help of TreeSet?

Edit

Using System.identityHashCode() is a great idea but there is (not so small) chance of collision.

Besides possibility of such a collision, there is one more pitfall. Suppose you have 3 objects: a, b, c such that map.get(a) = map.get(b) = map.get(c) (here = isn't assignment but the mathematical equality), identityHashCode(a) < identityHashCode(b) < identityHashCode(c), a.equals(c) is true, but a.equals(b) (and hence c.equals(b)) is false. After adding these 3 elements to a TreeSet in this order: a, b, c you can get into a situation when all of them have been added to the set, that contradicts the prescribed behaviour of the Set interface - it should not contain equal elements. How to deal with that?

In addition, it would be great if someone familiar with TreeSet mechanics explained to me what does the term "well-defined" in the phrase "The behavior of a set is well-defined even if its ordering is inconsistent with equals" from TreeSet javadoc mean.

26
  • 4
    A TreeSet works by keeping elements in a strict order. If you don't tell it how to order the elements, how can you expect it to be able to find them? Would you be able to find a book in a library where items are just placed randomly on shelves? Dec 11, 2018 at 22:56
  • @DawoodibnKareem Is the above comparator enough if the only thing that I need from TreeSet is to sort elements at the time of its creation and output the result as a list? I don't need extended possibilities like tailSet(). Dec 11, 2018 at 23:04
  • No, it really isn't. If your comparator doesn't work the way it should, your map won't work the way it should. Dec 11, 2018 at 23:06
  • The example you give for a comparator seems fairly arbitrary. If it really is arbitrary you may be better off using a LinkedHashSet (so that the order of insertions is maintained for iteration purposes)
    – Roy Shahaf
    Dec 16, 2018 at 19:38
  • 1
    To distinguish between "logically equal" but different objects in a TreeSet, I'd suggest using Guava's Ordering.arbitrary(). Dec 18, 2018 at 0:15

3 Answers 3

6

Unless you have an absolutely huge amount of Dummy objects and really bad luck, you can use System.identityHashCode()to break ties:

Comparator.<Dummy>comparingInt(d -> map.get(d))
          .thenComparingInt(System::identityHashCode)

Your comparator is not acceptable, since it violates the contract: you have d1 > d2 and d2 > d1 at the same time if they're not equal and don't share the same value in the map.

5
  • Thanks @John, fixed.
    – JB Nizet
    Dec 11, 2018 at 23:26
  • 2
    This is a step in the right direction, but I'd be wary of relying on uniqueness of identityHashCode. It's decidedly not unique. It's possible to get duplicate values after creating around 100,000 objects. Dec 15, 2018 at 5:38
  • @StuartMarks This is very important remark, thank you. I've tried to create a bunch of Dummy objects adding them to a list (to avoid them to be garbage collected) and adding their identityHashCodes to a set simultaneously. And already on the 105842-nd Dummy I've got a repeating hashCode. So this answer needs to be improved. Dec 16, 2018 at 15:51
  • I have edited the question, please look at the added paragraph. Dec 16, 2018 at 19:25
  • If you need to cope with the case when you have a collision in System::identityHashCode, you will need to arbitrarily break those ties (e.g. by the order the items appear in the input collection) and remember those decisions. Something like 1) map the input collection into a List using the above Comparator, then 2) create a new Comparator for the TreeSet based on List.indexOf from that list (or compute a map for more efficiency).
    – Rich
    Dec 20, 2018 at 15:04
3
+50

This answer covers just the first example in the question. The remainder of the question, and the various edits, are I think better answered as part of separate, focused questions.

The first example sets up 12 instances of Dummy, creates a map that maps each instance to an Integer in the range [0, 3], and then adds the 12 Dummy instances to a TreeSet. That TreeSet is provided with a comparator that uses the Dummy-to-Integer map. The result is that the TreeSet contains only four of the Dummy instances. The example concludes with the following statement:

The result is obviously unsatisfactory (contains less elements than the initial collection). This is because such a comparator is not consistent with equals, i.e. sometimes returns 0 for non-equal elements.

This last sentence is incorrect. The result contains fewer elements than were inserted because the comparator considers many of the instances to be duplicates and therefore they aren't inserted into the set. The equals method doesn't enter the discussion at all. Therefore, the concept of "consistent with equals" isn't relevant to this discussion. TreeSet never calls equals. The comparator is the only thing that determines membership in the TreeSet.

This seems like an unsatisfactory result, but only because we happen "know" that there are 12 distinct Dummy instances. However, the TreeSet doesn't "know" that they are distinct. It only knows how to compare the Dummy instances using the comparator. When it does so, it finds that several are duplicates. That is, the comparator returns 0 sometimes even though it's being called with Dummy instances that we believe to be distinct. That's why only four Dummy instances end up in the TreeSet.

I'm not entirely sure what the desired outcome is, but it seems like the result TreeSet should contain all 12 instances ordered by values in the Dummy-to-Integer map. My suggestion was to use Guava's Ordering.arbitrary() which provides a comparator that distinguishes between distinct-but-otherwise-equal elements, but does so in a way that satisfies the general contract of Comparator. If you create the TreeSet like this:

SortedSet<Dummy> sorted = new TreeSet<>(Comparator.<Dummy>comparingInt(map::get)
                                                  .thenComparing(Ordering.arbitrary()));

the result will be that the TreeSet contains all 12 Dummy instances, sorted by Integer value in the map, and with Dummy instances that map to the same value ordered arbitrarily.

In the comments, you stated that the Ordering.arbitrary doc "unequivocally cautions against using it in SortedSet". That's not quite right; that doc says,

Because the ordering is identity-based, it is not "consistent with Object.equals(Object)" as defined by Comparator. Use caution when building a SortedSet or SortedMap from it, as the resulting collection will not behave exactly according to spec.

The phrase "not behave exactly according to spec" really means that it will behave "strangely" as described in the class doc of Comparator:

The ordering imposed by a comparator c on a set of elements S is said to be consistent with equals if and only if c.compare(e1, e2)==0 has the same boolean value as e1.equals(e2) for every e1 and e2 in S.

Caution should be exercised when using a comparator capable of imposing an ordering inconsistent with equals to order a sorted set (or sorted map). Suppose a sorted set (or sorted map) with an explicit comparator c is used with elements (or keys) drawn from a set S. If the ordering imposed by c on S is inconsistent with equals, the sorted set (or sorted map) will behave "strangely." In particular the sorted set (or sorted map) will violate the general contract for set (or map), which is defined in terms of equals.

For example, suppose one adds two elements a and b such that (a.equals(b) && c.compare(a, b) != 0) to an empty TreeSet with comparator c. The second add operation will return true (and the size of the tree set will increase) because a and b are not equivalent from the tree set's perspective, even though this is contrary to the specification of the Set.add method.

You seemed to indicate that this "strange" behavior was unacceptable, in that Dummy elements that are equals shouldn't appear in the TreeSet. But the Dummy class doesn't override equals, so it seems like there's an additional requirement lurking behind here.

There are some additional questions added in later edits to the question, but as I mentioned above, I think these are better handled as separate question(s).

UPDATE 2018-12-22

After rereading the question edits and comments, I think I've finally figured out what you're looking for. You want a comparator over any object that provides a primary ordering based on some int-valued function that may result in duplicate values for unequal objects (as determined by the objects' equals method). Therefore, a secondary ordering is required that provides a total ordering over all unequal objects, but which returns zero for objects that are equals. This implies that the comparator should be consistent with equals.

Guava's Ordering.arbitrary comes close in that it provides an arbitrary total ordering over any objects, but it only returns zero for objects that are identical (that is, ==) but not for objects that are equals. It's thus inconsistent with equals.

It sounds, then, that you want a comparator that provides an arbitrary ordering over unequal objects. Here's a function that creates one:

static Comparator<Object> arbitraryUnequal() {
    Map<Object, Integer> map = new HashMap<>();
    return (o1, o2) -> Integer.compare(map.computeIfAbsent(o1, x -> map.size()),
                                       map.computeIfAbsent(o2, x -> map.size()));
}

Essentially, this assigns a sequence number to every newly seen unequal object and keeps these numbers in a map held by the comparator. It uses the map's size as the counter. Since objects are never removed from this map, the size and thus the sequence number always increases.

(If you intend for this comparator to be used concurrently, e.g., in a parallel sort, the HashMap should be replaced with a ConcurrentHashMap and the size trick should be modified to use an AtomicInteger that's incremented when new entries are added.)

Note that the map in this comparator builds up entries for every unequal object that it's ever seen. If this is attached to a TreeSet, objects will persist in the comparator's map even after they've been removed from the TreeSet. This is necessary so that if objects are added or removed, they'll retain consistent ordering over time. Guava's Ordering.arbitrary uses weak references to allow objects to be collected if they're no longer used. We can't do that, because we need to preserve the ordering of non-identical but equal objects.

You'd use it like this:

SortedSet<Dummy> sorted = new TreeSet<>(Comparator.<Dummy>comparingInt(map::get)
                                                  .thenComparing(arbitraryUnequal()));

You had also asked what "well-defined" means in the following:

The behavior of a set is well-defined even if its ordering is inconsistent with equals

Suppose you were to use a TreeSet using a comparator that's inconsistent with equals, such as the one using Guava's Ordering.arbitrary shown above. The TreeSet will still work as expected, consistent with itself. That is, it will maintain objects in a total ordering, it will not contain any two objects for which the comparator returns zero, and all its methods will work as specified. However, it is possible for there to be an object for which contains returns true (since that's computed using the comparator) but for which equals is false if called with the object actually in the set.

For example, BigDecimal is Comparable but its comparison method is inconsistent with equals:

> BigDecimal z = new BigDecimal("0.0")
> BigDecimal zz = new BigDecimal("0.00")
> z.compareTo(zz)
0
> z.equals(zz)
false
> TreeSet<BigDecimal> ts = new TreeSet<>()
> ts.add(z)
> HashSet<BigDecimal> hs = new HashSet<>(ts)
> hs.equals(ts)
true
> ts.contains(zz)
true
> hs.contains(zz)
false

This is what the spec means when it says things can behave "strangely". We have two sets that are equal. Yet they report different results for contains of the same object, and the TreeSet reports that it contains an object even though that object is unequal to an object in the set.

12
  • Thank you for the answer, it resolves the collision issue with JB Nizet's answer but is still incomplete. Please pay attention to the word like at the very beginning of the question. Do you really think that I had to sort 12 instances of such a simple class and disturbed the community with such a simple question, having chosen the reason "Question is widely applicable to a large audience" for the bounty? The paragraph For more real-life example imagine that... has existed from the outset. It explicitly (no "lurking behind") stipulates the possibility of the overridden equals() method. Dec 20, 2018 at 17:12
  • The requirement for the comparator is simple: it should be consistent with equals() (whether the latter is overridden or not) and distinguish elements that the initial comparator (map::get in your example) does not distinguish. Dec 20, 2018 at 17:23
  • @JohnMcClane Of course, the Dummy class is example code; everybody (including me) realizes that. What's difficult is that you're asking for a comparator that's consistent with equals, but you haven't described of what that equals method might do. It seems like equals might look at some fields not known to the comparator, yet you're requiring that the comparator distinguish objects (and provide a total ordering) based on those unknown fields. Seems impossible to me. Or, perhaps I'm still misunderstanding something. Dec 20, 2018 at 18:15
  • I'm not showing fields of the class instances of which are supposed to be sorted not due to secrecy of these fields but because I don't have them! I'm trying to sort type (T) instances of a generic class. The problem you described is common to generic classes: you have GenericClass<T extends Bound> and trying to make the most of it, but the only fields and methods that you can use when referring to T are those present in Bound. In my case, Bound = Object, so you have only methods from Object at your disposal. Dec 20, 2018 at 19:53
  • It is not a little, though. You have the method equals() itself. And don't forget about hashCode() which (as opposed to hapless identityHashCode()) you can assume to be consistent with equals(). Dec 20, 2018 at 19:54
0

Here's the comparator I ended up with. It is both reliable and memory efficient.

public static <T> Comparator<T> uniqualizer() {
    return new Comparator<T>() {
        private final Map<T, Integer> extraId = new HashMap<>();
        private int id;

        @Override
        public int compare(T o1, T o2) {
            int d = Integer.compare(o1.hashCode(), o2.hashCode());
            if (d != 0)
                return d;
            if (o1.equals(o2))
                return 0;
            d = extraId.computeIfAbsent(o1, key -> id++)
              - extraId.computeIfAbsent(o2, key -> id++);
            assert id > 0 : "ID overflow";
            assert d != 0 : "Concurrent modification";
            return d;
        }
    };
}

It creates total ordering on all objects of the given class T and thus allows to distinguish objects not distinguishable by a given comparator via attaching to it like this:

Comparator<T> partial = ...
Comparator<T> total = partial.thenComparing(uniqualizer());

In the example given at the question, T is Dummy and

partial = Comparator.<Dummy>comparingInt(map::get);

Note that you don't need to specify the type T when calling uniqualizer(), complier automatically determines it via type inference. You only have to make sure that hashCode() in T is consistent with equals(), as described in the general contract of hashCode(). Then uniqualizer() will give you the comparator (total) consistent with equals() and you can use it in any code that requires comparing objects of type T, e.g. when creating a TreeSet:

TreeSet<T> sorted = new TreeSet<>(total);

or sorting a list:

List<T> list = ...
Collections.sort(list, total);
4
  • What do you achieve with this? What problem do you solve? Dec 24, 2018 at 1:51
  • @ThorbjørnRavnAndersen I had to create SortedSet of objects of a generic type and the initial comparator (which I intended to use as a parameter to constructor of TreeSet) wasn't sufficient for this purpose. Dec 24, 2018 at 2:01
  • How does it make sense to sort a generic type? I am trying to understand why you were forced to basically create a sort key on the fly. Dec 24, 2018 at 2:07
  • @ThorbjørnRavnAndersen That was done to make TreeSet work as a normal set, i.e. to accept distinct (by equals()) and discard repeating elements, to give expected results of contains() etc., while using its sorting capabilities at the same time. The initial sorting criteria was suitable for a list, but not for a set. The key point here is that the sorting criteria was external, i.e. not determined by the input objects' fields themselves. Dec 24, 2018 at 2:22

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.