From a3b3b74ec6325cd19169fdbaea26f27a96a72fea Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Tue, 27 Jan 2015 15:03:22 +0000 Subject: [PATCH] [COLLECTIONS-426] Revert performance improvement and add clarifying javadoc wrt runtime complexity. git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1655060 13f79535-47bb-0310-9956-ffa450edef68 --- src/changes/changes.xml | 4 +++ .../collections4/set/ListOrderedSet.java | 28 +++++++++++-------- .../collections4/set/ListOrderedSetTest.java | 25 ----------------- 3 files changed, 20 insertions(+), 37 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index df4fe22ed..c67d6d285 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -22,6 +22,10 @@ + + Reverted performance improvement for "ListOrderedSet#retainAll(Collection)" + introduced in 4.0. Added clarifying javadoc wrt runtime complexity instead. + Added a Builder for "PredicatedCollection". Elements added to the builder that fail the predicate will not throw an IllegalArgumentException. The builder diff --git a/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java b/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java index 48356fc7e..b4a476ced 100644 --- a/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java +++ b/src/main/java/org/apache/commons/collections4/set/ListOrderedSet.java @@ -225,27 +225,31 @@ public class ListOrderedSet return result; } + /** + * {@inheritDoc} + *

+ * This implementation iterates over the elements of this set, checking + * each element in turn to see if it's contained in coll. + * If it's not contained, it's removed from this set. As a consequence, + * it is advised to use a collection type for coll that provides + * a fast (e.g. O(1)) implementation of {@link Collection#contains(Object)}. + */ @Override public boolean retainAll(final Collection coll) { - final Set collectionRetainAll = new HashSet(); - for (final Object next : coll) { - if (decorated().contains(next)) { - collectionRetainAll.add(next); - } - } - if (collectionRetainAll.size() == decorated().size()) { + boolean result = decorated().retainAll(coll); + if (result == false) { return false; } - if (collectionRetainAll.size() == 0) { - clear(); + if (decorated().size() == 0) { + setOrder.clear(); } else { - for (final Iterator it = iterator(); it.hasNext();) { - if (!collectionRetainAll.contains(it.next())) { + for (Iterator it = setOrder.iterator(); it.hasNext();) { + if (!decorated().contains(it.next())) { it.remove(); } } } - return true; + return result; } @Override diff --git a/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java b/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java index e898c20f9..ba6b0d5c0 100644 --- a/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java +++ b/src/test/java/org/apache/commons/collections4/set/ListOrderedSetTest.java @@ -204,31 +204,6 @@ public class ListOrderedSetTest assertEquals(Integer.valueOf(0), orderedSet.get(4)); } - /* - * test case for https://issues.apache.org/jira/browse/COLLECTIONS-426 - */ - @SuppressWarnings("boxing") // OK in test code - public void testRetainAllCollections426() { - final int size = 100000; - final ListOrderedSet set = new ListOrderedSet(); - for (int i = 0; i < size; i++) { - set.add(i); - } - final ArrayList list = new ArrayList(); - for (int i = size; i < 2 * size; i++) { - list.add(i); - } - - final long start = System.currentTimeMillis(); - set.retainAll(list); - final long stop = System.currentTimeMillis(); - - // make sure retainAll completes under 5 seconds - // TODO if test is migrated to JUnit 4, add a Timeout rule. - // http://kentbeck.github.com/junit/javadoc/latest/org/junit/rules/Timeout.html - assertTrue(stop - start < 5000); - } - @SuppressWarnings("unchecked") public void testDuplicates() { final List list = new ArrayList(10);