Java Set strange behaviour

Question

I had a problem in which I need to change the comparable value of a sorted set based on some condition.

Doing something like this:

SortedSet<T> groups;

for(T t: groups){
        t.setOrdinal(max);
}

Will not automatically rearrange the SortedSet.

Reading online I figured out that I need to remove the object from the set and then add it again. Obviously, I cannot do it while iterating over the set. So I decided to make a Arraylist of the set. Make the set null and then add all the elements again so that they follow the sorted order. I did something like this:

SortedSet groups;

List<T> groupList = new ArrayList<T>(groups);
groups = null;
for(T t: groupList){
        t.setOrdinal(max);
}

groups = new TreeSet<T>(groupList);

But when I check the groups set it didnt follow the sort order based on comparator which compared the ordinal of the object T

But when I tried something like this:

SortedSet groups;

List<T> groupList = new ArrayList<T>(groups);
groups.clear();
for(T t: groupList){
        t.setOrdinal(max);
}

groups.addAll(groupList);

I got the result as expected. Can someone explain me whats happening?

This is how I have implemented my class T

public class T implements Serializable, Comparable<T> {
//
int ordinal;
//getter
//setter

 @Override
  public int compareTo(T that) {
    if (that == null) {
      return 1;
    }

    return this.ordinal - that.ordinal;
  }

}

For those want to see complete program:

List<SmartUser> groupsList = new ArrayList<SmartUser>(groups);
groups = null;
for (SmartUser smartUser : groupsList) {
        if (smartUser.isExpired()) {
                smartUser.setOrdinal(Long.MAX_VALUE);
        }
        SmartUserQuery smartUserQuery = smartUser.getSmartUserQuery();
        if (smartUserQuery != null) {
                //Do parallel processing: of each samrtUser
        }
}

groups = new TreeSet<SmartUser>groupsList;

Correct result giving approach:

List<SmartUser> groupsList = new ArrayList<SmartUser>(groups);
groups.clear();
for (SmartUser smartUser : groupsList) {
        if (smartUser.isExpired()) {
                smartUser.setOrdinal(Long.MAX_VALUE);
        }
        SmartUserQuery smartUserQuery = smartUser.getSmartUserQuery();
        if (smartUserQuery != null) {
                //Do parallel processing: of each samrtUser
        }
}

groups.addAll(groupsList);

Thanks.

Please show a short but *complete* program demonstrating the problem. For example, we don't know if your `SortedSet` used a custom comparator or the normal `Comparable` implementation. — Jon Skeet, Oct 09 '12 at 15:50
From the way that you presented it, both ways should have worked just fine. — Sergey Kalinichenko, Oct 09 '12 at 15:52
Aside from anything else, your `compareTo` method is broken due to overflow... and the sample you've shown would set the ordinal of *every* value to the same value, so any order is acceptable. Again, short but complete program please... — Jon Skeet, Oct 09 '12 at 15:55
Could you describe what you're actually trying to do with this? Trying to squeeze this into a `SortedSet` is only going to end up with fragile, easily broken code. — Louis Wasserman, Oct 09 '12 at 18:23
I know this one is old, but still listed as unanswered. Would you please accept and upvote my answer if it seems appropriate? Thanks. — kriegaex, Jun 09 '14 at 12:10

Amit Deshpande · Answer 1 · 2012-10-09T16:24:46.917

Change your compareTo method to below

@Override
public int compareTo(CustomObject o) {
    return Integer.valueOf(o.ordinal).compareTo(ordinal );
}

Also as per TreeSet contract you should provide equals method which will be in sync with compareTo method

 @Override
public boolean equals(Object obj) {
    if (obj == null)
        return false;
    if (!(obj instanceof CustomObject))
        return false;
    CustomObject o = (CustomObject) obj;

    return this.ordinal == o.ordinal;
}

A Sample implementation

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

public class CustomObject implements Comparable<CustomObject> {

private int ordinal = 0;

public CustomObject(int priority) {
    this.ordinal = priority;
}

/**
 * @param args
 */
public static void main(String[] args) {

    List<CustomObject> list = new ArrayList<CustomObject>();
    list.add(new CustomObject(1));
    list.add(new CustomObject(2));
    list.add(new CustomObject(3));
    list.add(new CustomObject(6));
    list.add(new CustomObject(4));
    System.out.println("Before: " + list);
    for (CustomObject object : list) {
        if (object.ordinal == 4) {
            object.ordinal = 10;
        }
    }
    Set<CustomObject> set = new TreeSet<CustomObject>();
    set.addAll(list);
    System.out.println("After: " + set);

}

@Override
public int compareTo(CustomObject o) {
    return Integer.valueOf(o.ordinal).compareTo(ordinal);
}

@Override
public boolean equals(Object obj) {
    if (obj == null)
        return false;
    if (!(obj instanceof CustomObject))
        return false;
    CustomObject o = (CustomObject) obj;

    return this.ordinal == o.ordinal;
}

@Override
public String toString() {
    return " Ordinal Value is :" + ordinal;
}

}

you do need to override the .equals method, but that also means you should override the .hashcode method as well or your whole operation will break down. Hashsets operate with an internal hashmap, and if the .equals method and .hashcode method aren't consistent (ie whatever fields are used in determining equality need to be part of your hashcode generation), the data structure breaks down. In your case, you can delegate the method to Integer.valueOf(ordinal).hashcode() — Matt, Oct 09 '12 at 20:06
@Matt Question was about TreeSet so I did now include hashCode since hashing is required only for HashSet and HashMap. But yes it is good to have and it is easy to add. — Amit Deshpande, Oct 10 '12 at 06:47
I think my compareTo method is fine because I'm sure that the ordinal will always be non - negative. — Global Warrior, Oct 10 '12 at 17:51

score 0 · Answer 2 · edited May 23 '17 at 10:25

0

I figured out that I need to remove the object from the set and then add it again. Obviously, I cannot do it while iterating over the set. So I decided to make a Arraylist of the set.

Take a look at my answer at maintaining TreeSet sort as object changes value and try my UpdateableTreeSet implementation. It permits you to do deferred updates while iterating over the sorted set.

edited May 23 '17 at 10:25

Community

1
1

answered Oct 27 '12 at 10:44

kriegaex

63,017
15
111
202

Java Set strange behaviour

2 Answers2