From e7ab1162c237f96bee0c2cdb0318c3b773a41657 Mon Sep 17 00:00:00 2001 From: Stephen Kestle Date: Sun, 11 Nov 2007 21:44:41 +0000 Subject: [PATCH] Applied (most of) Brian Egge's clean up patch of previous commit for COLLECTIONS-245 - thanks! (And we're at 100%) git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/branches/collections_jdk5_branch@593964 13f79535-47bb-0310-9956-ffa450edef68 --- .../commons/collections/CollectionUtils.java | 86 +++++++--------- .../collections/TestCollectionUtils.java | 99 +++++++++++++++---- 2 files changed, 117 insertions(+), 68 deletions(-) diff --git a/src/java/org/apache/commons/collections/CollectionUtils.java b/src/java/org/apache/commons/collections/CollectionUtils.java index 70b2cb046..32171c279 100644 --- a/src/java/org/apache/commons/collections/CollectionUtils.java +++ b/src/java/org/apache/commons/collections/CollectionUtils.java @@ -122,16 +122,14 @@ public class CollectionUtils { } - /** Constant to avoid repeated object creation */ - private static Integer INTEGER_ONE = new Integer(1); - /** * An empty unmodifiable collection. * The JDK provides empty Set and List implementations which could be used for * this purpose. However they could be cast to Set or List which might be * undesirable. This implementation only implements Collection. */ - public static final Collection EMPTY_COLLECTION = UnmodifiableCollection.decorate(new ArrayList()); + @SuppressWarnings("unchecked") + public static final Collection EMPTY_COLLECTION = UnmodifiableCollection.decorate(new ArrayList()); /** * CollectionUtils should not normally be instantiated. @@ -144,6 +142,7 @@ public class CollectionUtils { * * @see #EMPTY_COLLECTION * @since 4.0 + * @return immutable empty collection */ @SuppressWarnings("unchecked") public static Collection emptyCollection() { @@ -162,8 +161,6 @@ public class CollectionUtils { * @param b the second collection, must not be null * @param the generic type that is able to represent the types contained * in both input collections. - * @param the generic type of the first input {@link Iterable}. - * @param the generic type of the second input {@link Iterable}. * @return the union of the two collections * @see Collection#addAll */ @@ -187,8 +184,6 @@ public class CollectionUtils { * @param b the second collection, must not be null * @param the generic type that is able to represent the types contained * in both input collections. - * @param the generic type of the first input {@link Iterable}. - * @param the generic type of the second input {@link Iterable}. * @return the intersection of the two collections * @see Collection#retainAll * @see #containsAny @@ -218,8 +213,6 @@ public class CollectionUtils { * @param b the second collection, must not be null * @param the generic type that is able to represent the types contained * in both input collections. - * @param the generic type of the first input {@link Iterable}. - * @param the generic type of the second input {@link Iterable}. * @return the symmetric difference of the two collections */ public static Collection disjunction(final Iterable a, final Iterable b) { @@ -240,8 +233,6 @@ public class CollectionUtils { * @param b the collection to subtract, must not be null * @param the generic type that is able to represent the types contained * in both input collections. - * @param the generic type of the first input {@link Iterable}. - * @param the generic type of the second input {@link Iterable}. * @return a new collection with the results * @see Collection#removeAll */ @@ -266,16 +257,16 @@ public class CollectionUtils { * @since 2.1 * @see #intersection */ - public static boolean containsAny(final Collection coll1, final Collection coll2) { + public static boolean containsAny(final Collection coll1, final Collection coll2) { if (coll1.size() < coll2.size()) { - for (Iterator it = coll1.iterator(); it.hasNext();) { - if (coll2.contains(it.next())) { + for (Object aColl1 : coll1) { + if (coll2.contains(aColl1)) { return true; } } } else { - for (Iterator it = coll2.iterator(); it.hasNext();) { - if (coll1.contains(it.next())) { + for (Object aColl2 : coll2) { + if (coll1.contains(aColl2)) { return true; } } @@ -306,7 +297,7 @@ public class CollectionUtils { for (I obj : coll) { Integer c = count.get(obj); if (c == null) { - count.put(obj, INTEGER_ONE); + count.put(obj, 1); } else { count.put(obj, c + 1); } @@ -474,7 +465,7 @@ public class CollectionUtils { public static void filter(Iterable collection, Predicate predicate) { if (collection != null && predicate != null) { for (Iterator it = collection.iterator(); it.hasNext();) { - if (predicate.evaluate(it.next()) == false) { + if (!predicate.evaluate(it.next())) { it.remove(); } } @@ -593,6 +584,7 @@ public class CollectionUtils { * the predicate to use, may be null * @param outputCollection * the collection to output into, may not be null + * @return outputCollection */ public static Collection select(Collection inputCollection, Predicate predicate, Collection outputCollection) { if (inputCollection != null && predicate != null) { @@ -637,11 +629,12 @@ public class CollectionUtils { * the predicate to use, may be null * @param outputCollection * the collection to output into, may not be null + * @return outputCollection */ public static Collection selectRejected(Collection inputCollection, Predicate predicate, Collection outputCollection) { if (inputCollection != null && predicate != null) { for (I item : inputCollection) { - if (predicate.evaluate(item) == false) { + if (!predicate.evaluate(item)) { outputCollection.add(item); } } @@ -661,7 +654,6 @@ public class CollectionUtils { * the transformer to use, may be null * @param the type of object in the input collection * @param the type of object in the output collection - * @param the output type of the transformer - this extends O. * @return the transformed result (new list) * @throws NullPointerException * if the input collection is null @@ -685,7 +677,6 @@ public class CollectionUtils { * the transformer to use, may be null * @param the type of object in the input collection * @param the type of object in the output collection - * @param the output type of the transformer - this extends O. * @return the transformed result (new list) */ public static Collection collect(Iterator inputIterator, Transformer transformer) { @@ -772,9 +763,9 @@ public class CollectionUtils { * @throws NullPointerException * if the collection or iterator is null */ - public static boolean addAll(Collection collection, Iterable iterable) { + public static boolean addAll(Collection collection, Iterable iterable) { if (iterable instanceof Collection) { - return collection.addAll((Collection) iterable); + return collection.addAll((Collection) iterable); } return addAll(collection, iterable.iterator()); } @@ -843,15 +834,16 @@ public class CollectionUtils { * @throws IllegalArgumentException if the object type is invalid */ public static T get(Iterator iterator, int index) { - checkIndexBounds(index); + int i = index; + checkIndexBounds(i); while (iterator.hasNext()) { - index--; - if (index == -1) { + i--; + if (i == -1) { return iterator.next(); } iterator.next(); } - throw new IndexOutOfBoundsException("Entry does not exist: " + index); + throw new IndexOutOfBoundsException("Entry does not exist: " + i); } /** @@ -916,45 +908,45 @@ public class CollectionUtils { * @throws IllegalArgumentException if the object type is invalid */ public static Object get(Object object, int index) { - if (index < 0) { - throw new IndexOutOfBoundsException("Index cannot be negative: " + index); + int i = index; + if (i < 0) { + throw new IndexOutOfBoundsException("Index cannot be negative: " + i); } if (object instanceof Map) { Map map = (Map) object; Iterator iterator = map.entrySet().iterator(); - return get(iterator, index); + return get(iterator, i); } else if (object instanceof Object[]) { - return ((Object[]) object)[index]; + return ((Object[]) object)[i]; } else if (object instanceof Iterator) { Iterator it = (Iterator) object; while (it.hasNext()) { - index--; - if (index == -1) { + i--; + if (i == -1) { return it.next(); - } else { - it.next(); } + it.next(); } - throw new IndexOutOfBoundsException("Entry does not exist: " + index); + throw new IndexOutOfBoundsException("Entry does not exist: " + i); } else if (object instanceof Collection) { Iterator iterator = ((Collection) object).iterator(); - return get(iterator, index); + return get(iterator, i); } else if (object instanceof Enumeration) { Enumeration it = (Enumeration) object; while (it.hasMoreElements()) { index--; - if (index == -1) { + if (i == -1) { return it.nextElement(); } else { it.nextElement(); } } - throw new IndexOutOfBoundsException("Entry does not exist: " + index); + throw new IndexOutOfBoundsException("Entry does not exist: " + i); } else if (object == null) { throw new IllegalArgumentException("Unsupported object type: null"); } else { try { - return Array.get(object, index); + return Array.get(object, i); } catch (IllegalArgumentException ex) { throw new IllegalArgumentException("Unsupported object type: " + object.getClass().getName()); } @@ -1113,14 +1105,6 @@ public class CollectionUtils { } } - private static final int getFreq(final T obj, final Map freqMap) { - Integer count = freqMap.get(obj); - if (count != null) { - return count.intValue(); - } - return 0; - } - /** * Returns true if no more elements can be added to the Collection. *

@@ -1246,7 +1230,7 @@ public class CollectionUtils { * @return a synchronized collection backed by the given collection * @throws IllegalArgumentException if the collection is null */ - public static Collection synchronizedCollection(Collection collection) { + public static Collection synchronizedCollection(Collection collection) { return SynchronizedCollection.decorate(collection); } @@ -1259,7 +1243,7 @@ public class CollectionUtils { * @return an unmodifiable collection backed by the given collection * @throws IllegalArgumentException if the collection is null */ - public static Collection unmodifiableCollection(Collection collection) { + public static Collection unmodifiableCollection(Collection collection) { return UnmodifiableCollection.decorate(collection); } diff --git a/src/test/org/apache/commons/collections/TestCollectionUtils.java b/src/test/org/apache/commons/collections/TestCollectionUtils.java index 993eb0408..3189c8461 100644 --- a/src/test/org/apache/commons/collections/TestCollectionUtils.java +++ b/src/test/org/apache/commons/collections/TestCollectionUtils.java @@ -22,20 +22,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; -import java.util.Vector; +import java.util.*; import org.apache.commons.collections.bag.HashBag; import org.apache.commons.collections.buffer.BoundedFifoBuffer; @@ -77,7 +64,7 @@ public class TestCollectionUtils extends MockTestCase { private List collectionB = null; /** - * Collection of {@link Integers}s that are equivalent to the Longs in + * Collection of {@link Integer}s that are equivalent to the Longs in * collectionB. */ private Collection collectionC = null; @@ -93,7 +80,7 @@ public class TestCollectionUtils extends MockTestCase { private Collection collectionB2 = null; /** - * Collection of {@link Integers}s (cast as {@link Number}s) that are + * Collection of {@link Integer}s (cast as {@link Number}s) that are * equivalent to the Longs in collectionB. */ private Collection collectionC2 = null; @@ -439,6 +426,16 @@ public class TestCollectionUtils extends MockTestCase { assertTrue(!CollectionUtils.isEqualCollection(collectionC, collectionA)); } + @Test + public void testIsEqualCollectionReturnsFalse() { + List b = new ArrayList(collectionA); + // remove an extra '2', and add a 5. This will increase the size of the cardinality + b.remove(1); + b.add(5); + assertFalse(CollectionUtils.isEqualCollection(collectionA, b)); + assertFalse(CollectionUtils.isEqualCollection(b, collectionA)); + } + @Test public void testIsEqualCollection2() { Collection a = new ArrayList(); @@ -1073,7 +1070,7 @@ public class TestCollectionUtils extends MockTestCase { // expected } try { - collection = CollectionUtils.predicatedCollection(null, predicate); + CollectionUtils.predicatedCollection(null, predicate); fail("Expecting IllegalArgumentException for null collection."); } catch (IllegalArgumentException ex) { // expected @@ -1108,6 +1105,12 @@ public class TestCollectionUtils extends MockTestCase { assertEquals(true, CollectionUtils.isFull(buf2)); } + @Test + public void isEmpty() { + assertFalse(CollectionUtils.isNotEmpty(null)); + assertTrue(CollectionUtils.isNotEmpty(collectionA)); + } + @Test public void maxSize() { Set set = new HashSet(); @@ -1347,6 +1350,68 @@ public class TestCollectionUtils extends MockTestCase { verify(); } + @Test + public void addAllForEnumeration() { + Hashtable h = new Hashtable(); + h.put(5, 5); + Enumeration enumeration = h.keys(); + CollectionUtils.addAll(collectionA, enumeration); + assertTrue(collectionA.contains(5)); + } + + @Test + public void addAllForElements() { + CollectionUtils.addAll(collectionA, new Integer[]{5}); + assertTrue(collectionA.contains(5)); + } + + @Test + public void get() { + try { + CollectionUtils.get((Object)collectionA, -3); + fail(); + } catch(IndexOutOfBoundsException e) { + ; + } + try { + CollectionUtils.get((Object)collectionA.iterator(), 30); + fail(); + } catch(IndexOutOfBoundsException e) { + ; + } + try { + CollectionUtils.get((Object)null, 0); + fail(); + } catch(IllegalArgumentException e) { + ; + } + assertEquals(2, CollectionUtils.get((Object)collectionA, 2)); + assertEquals(2, CollectionUtils.get((Object)collectionA.iterator(), 2)); + Map map = CollectionUtils.getCardinalityMap(collectionA); + assertEquals(map.entrySet().iterator().next(), CollectionUtils.get( + (Object)map, 0)); + } + + /** + * TODO: Should {@link CollectionUtils} be able to be extended? If it is extended, subclasses must 'override' the static methods with + * call-throughs anyhow, otherwise java compiler warnings will result + */ + @Test + public void ensureCollectionUtilsCanBeExtended() { + new CollectionUtils() {}; + } + + @Test + public void reverse() { + CollectionUtils.reverseArray(new Object[] {}); + Integer[] a = collectionA.toArray(new Integer[collectionA.size()]); + CollectionUtils.reverseArray(a); + // assume our implementation is correct if it returns the same order as the Java function + Collections.reverse(collectionA); + assertEquals(collectionA, Arrays.asList(a)); + } + + /** * Records the next object returned for a mock iterator */