No, it's the current implementation that is technically wrong. Pair.java defines equality as equality of both keys and values. Comparator orderings are supposed to be consistent with equals() ( http://docs.oracle.com/javase/7/docs/api/java/util/Comparator.html). That is, two Pairs should be equal if and only if they compare to 0. If you only compare keys in an ordering, you will compare to 0 for un-equal Pairs sometimes. So defining ordering just on keys can't match this definition.
This means you could start from different orderings of the same Pairs in a List, sort them, and not get the same result. It means that in theory some classes like TreeSet won't work with Pairs and the current Comparator. In practice? who knows. I bet it doesn't matter for this use case. I prefer correctness over speed as anyone would, and thought it was a decent motivation for this addition -- hey, even CM is a customer for this class, let alone external consumers. I sense you don't want to commit it, so I'd say let's just close the issue, having committed the toString() and factory method called create(). Thanks for that much, I didn't mean to take nearly this much time. On Wed, Oct 16, 2013 at 10:26 PM, Gilles <gil...@harfang.homelinux.org>wrote: > > Hmm, I now wonder whether it is at all correct to add the comparison of > values > in that case (and I start to wonder why the tests still pass). > >
