Is it ok to change a value that is used in compareTo in a TreeSet?

Question

I have this simple Pojo:

public static class Pojo implements Comparable<Pojo> {

    Integer value;

    @Override
    public int compareTo(Pojo o) {
        return value.compareTo(o.value);
    }

    // getters and setter ommited

}

Is it ok to change value of a Pojo while it's in a TreeSet? I don't mind if this breaks the sorting in the TreeSet.

I'm curious because changing value used in a hashCode() or equas() while the instance is in a HashMap is not a good idea.

Answer 1

If we change attributes that influence the element order, we violate contracts of TreeSet.

As per TreeSet's documentation, it

... provides guaranteed log(n) time cost for the basic operations (add, remove and contains).

The TreeSet can guarantee those time costs by keeping the set ordered. If we, however, change attributes of elements, that would in turn change the ordering within the set, we break this contract. The TreeSet does not constantly "monitor" all elements for changes. Furthermore, a change could trigger a reorder, which would have costs of n * log(n) (with n being the current size of the TreeSet).

We can make the effects of this form of contract violation visible with the following code:

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.TreeSet;

class Ideone {
    public static void main(String[] args) {
        Foo fooOne = Foo.of(1);
        Foo fooTwo = Foo.of(2);
        Foo fooThree = Foo.of(3);
        TreeSet<Foo> set = new TreeSet<>();
        set.addAll(List.of(fooOne, fooTwo, fooThree));
        System.out.println(set.contains(fooOne));
        fooOne.setValue(4);
        System.out.println(set.contains(fooOne));
        System.out.println(new ArrayList<>(set).contains(fooOne));
    }
}

class Foo implements Comparable<Foo> {
    private int value;

    private Foo(int value) {
        this.setValue(value);
    }

    public static Foo of(int value) {
        return new Foo(value);
    }

    public int getValue() {
        return value;
    }

    public Foo setValue(int value) {
        this.value = value;
        return this;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) {
            return true;
        }
        if (o == null || getClass() != o.getClass()) {
            return false;
        }
        Foo foo = (Foo) o;
        return getValue() == foo.getValue();
    }

    @Override
    public int hashCode() {
        return Objects.hash(value);
    }

    @Override
    public int compareTo(Foo that) {
        return getValue() - that.getValue();
    }
}

Ideone demo

If the TreeSet would reorder on element attribute changes, all three lines should show true. However, since we changed fooOne's value from 1 to 4 (hence it would be expected to be the last element in the set), the second contains(fooOne) will return false since the TreeSet is no longer sorted. Converting the TreeSet into an ArrayList and searching within the ArrayList, however, yields the expected result since the ArrayList does not make any assumption about element order.

If the guaranteed log(n) time cost for basic operation is not essential to the use case, I would suggest using a different data structure (e.g. a List) that does not rely on element ordering. Keep in mind, however, that this comes at a higher time cost for (at least some of the) basic operations.

EDIT:

As was mentioned by @Scratte in the comments, we can even break the TreeSet on a more fundamental level by adding the (modified, but still identical wrt. ==) fooOne to the TreeSet, increasing its size by one:

class Ideone {
    public static void main(String[] args) {
        Foo fooOne = Foo.of(1);
        Foo fooTwo = Foo.of(2);
        Foo fooThree = Foo.of(3);
        TreeSet<Foo> set = new TreeSet<>();
        set.addAll(List.of(fooOne, fooTwo, fooThree));
        System.out.println(set.size());
        fooOne.setValue(4);
        set.add(fooOne);
        System.out.println(set.size());
    }
}

Ideone demo