Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it OK to have a Java Comparator where order can change dynamically?

I have a set of time stamped values I'd like to place in a sorted set.

public class TimedValue {
    public Date time;
    public double value;

    public TimedValue(Date time, double value) {
        this.time = time;
        this.value = value;
    }
}

The business logic for sorting this set says that values must be ordered in descending value order, unless it's more than 7 days older than the newest value.

So as a test, I came up with the following code...

DateFormat dateFormatter = new SimpleDateFormat("MM/dd/yyyy");
TreeSet<TimedValue> mySet = new TreeSet<TimedValue>(new DateAwareComparator());
mySet.add(new TimedValue(dateFormatter.parse("01/01/2009"), 4.0 )); // too old
mySet.add(new TimedValue(dateFormatter.parse("01/03/2009"), 3.0)); // Most relevant
mySet.add(new TimedValue(dateFormatter.parse("01/09/2009"), 2.0));

As you can see, initially the first value is more relevant than the second, but once the final value is added to the set, the first value has expired and should be the least relevant.

My initial tests say that this should work... that the TreeSet will dynamically reorder the entire list as more values are added.

But even though I see it, I'm not sure I believe it.

Will a sorted collection reorder the entire set as each element is added? Are there any gotchas to using a sorted collection in this manner (i.e performance)? Would it be better to manually sort the list after all values have been added (I'm guessing it would be)?



Follow-up: As many (and even I to a certain extent) suspected, the sorted collection does not support this manner of "dynamic reordering". I believe my initial test was "working" quite by accident. As I added more elements to the set, the "order" broke down quite rapidly. Thanks for all the great responses, I refactored my code to use approaches suggested by many of you.

like image 360
Vinnie Avatar asked Nov 28 '22 12:11

Vinnie


2 Answers

I don't see how your comparator can even detect the change, unless it remembers the newest value it's currently seen - and that sounds like an approach which is bound to end in tears.

I suggest you do something along the following lines:

  • Collect your data in an unordered set (or list)
  • Find the newest value
  • Create a comparator based on that value, such that all comparisons using that comparator will be fixed (i.e. it will never return a different result based on the same input values; the comparator itself is immutable although it depends on the value originally provided in the constructor)
  • Create a sorted collection using that comparator (in whatever way seems best depending on what you then want to do with it)
like image 55
Jon Skeet Avatar answered Dec 05 '22 10:12

Jon Skeet


I would advise against this for a few reasons:

  1. Since it's basically a red-black tree behind the scenes (which doesn't necessarily have to be rebuilt from scratch on every insertion), you might easily end up with values in the wrong part of the tree (invalidating most of the TreeSet API).
  2. The behavior is not defined in the spec, and thus may change later even if it's working now.
  3. In the future, when anything goes strangely wrong in anything remotely touching this code, you'll spend time suspecting that this is the cause.

I would recommend either recreating/resorting the TreeSet before searching it or (my preference) iterating through the set before the search and removing any of the objects that are too old. You could even, if you wanted to trade some memory for speed, keep a second list ordered by date and backed by the same objects so that you all you would have to do to filter your TreeSet is remove objects from the TreeSet based on the time-sorted list.

like image 20
Austin Mills Avatar answered Dec 05 '22 09:12

Austin Mills