Skip to content

Commit 7ef595c

Browse files
committed
Rework uniquify to not use iterators
CollectionUtils.uniquify is based on C++ std::unique. However, C++ iterators are not quite the same as Java iterators. In particular, advancing them only allows grabbing the value once. This commit reworks uniquify to be based on list indices instead of iterators. closes #126883
1 parent 85cb5f9 commit 7ef595c

File tree

2 files changed

+13
-10
lines changed

2 files changed

+13
-10
lines changed

server/src/main/java/org/elasticsearch/common/util/CollectionUtils.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,20 +51,22 @@ public static <T> void uniquify(List<T> list, Comparator<T> cmp) {
5151
return;
5252
}
5353

54-
ListIterator<T> uniqueItr = list.listIterator();
55-
ListIterator<T> existingItr = list.listIterator();
56-
T uniqueValue = uniqueItr.next(); // get first element to compare with
57-
existingItr.next(); // advance the existing iterator to the second element, where we will begin comparing
54+
int resultIndex = 0;
55+
int currentIndex = 1;
56+
T lastValue = list.get(resultIndex); // get first element to compare with
5857
do {
59-
T existingValue = existingItr.next();
60-
if (cmp.compare(existingValue, uniqueValue) != 0 && (uniqueValue = uniqueItr.next()) != existingValue) {
61-
uniqueItr.set(existingValue);
58+
T currentValue = list.get(currentIndex);
59+
if (cmp.compare(lastValue, currentValue) != 0) {
60+
lastValue = currentValue;
61+
if (++resultIndex != currentIndex) {
62+
list.set(resultIndex, currentValue);
63+
}
6264
}
63-
} while (existingItr.hasNext());
65+
} while (++currentIndex < list.size());
6466

6567
// Lop off the rest of the list. Note with LinkedList this requires advancing back to this index,
6668
// but Java provides no way to efficiently remove from the end of a non random-access list.
67-
list.subList(uniqueItr.nextIndex(), list.size()).clear();
69+
list.subList(resultIndex + 1, list.size()).clear();
6870
}
6971

7072
/**

server/src/test/java/org/elasticsearch/common/util/CollectionUtilsTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ private <T> void assertUniquify(List<T> list, Comparator<T> cmp, int size) {
6565
for (List<T> listCopy : List.of(new ArrayList<T>(list), new LinkedList<T>(list))) {
6666
CollectionUtils.uniquify(listCopy, cmp);
6767
for (int i = 0; i < listCopy.size() - 1; ++i) {
68-
assertThat(cmp.compare(listCopy.get(i), listCopy.get(i + 1)), lessThan(0));
68+
assertThat(listCopy.get(i) + " < " + listCopy.get(i + 1), cmp.compare(listCopy.get(i), listCopy.get(i + 1)), lessThan(0));
6969
}
7070
assertThat(listCopy.size(), equalTo(size));
7171
}
@@ -78,6 +78,7 @@ public void testUniquify() {
7878
assertUniquify(List.of(1, 1, 1), Comparator.naturalOrder(), 1);
7979
assertUniquify(List.of(1, 2, 2, 3), Comparator.naturalOrder(), 3);
8080
assertUniquify(List.of(1, 2, 2, 2), Comparator.naturalOrder(), 2);
81+
assertUniquify(List.of(1, 2, 2, 3, 3, 5), Comparator.naturalOrder(), 4);
8182
}
8283

8384
public void testEmptyPartition() {

0 commit comments

Comments
 (0)