commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephen Kestle <stephen.kes...@orionhealth.com>
Subject Edit: Re: svn commit: r593964 - in /commons/proper/collections/branches/collections_jdk5_branch/src: java/org/apache/commons/collections/CollectionUtils.java test/org/apache/commons/collections/TestCollectionUtils.java
Date Sun, 11 Nov 2007 22:02:25 GMT
Good patch - seems I need to update my compiler warnings a bit.

Nice to get to 100% - one thing that interested me about that was
testing the extension of CollectionUtils.  People - please see
https://issues.apache.org/jira/browse/COLLECTIONS-276.

And while CollectionUtils is updated for generics, I haven't fixed the
compiler warnings in a few methods (get() for example) as I think these
need to be thought through a bit more - does it still make sense to have
a get(Object, int)?  Possibly - could it be get(Iterable, int)?

Cheers

Stephen

skestle@apache.org wrote:
> Author: skestle
> Date: Sun Nov 11 13:44:41 2007
> New Revision: 593964
>
> URL: http://svn.apache.org/viewvc?rev=593964&view=rev
> Log:
> Applied (most of) Brian Egge's clean up patch of previous commit for COLLECTIONS-245
- thanks! (And we're at 100%)
>
> Modified:
>     commons/proper/collections/branches/collections_jdk5_branch/src/java/org/apache/commons/collections/CollectionUtils.java
>     commons/proper/collections/branches/collections_jdk5_branch/src/test/org/apache/commons/collections/TestCollectionUtils.java
>
> Modified: commons/proper/collections/branches/collections_jdk5_branch/src/java/org/apache/commons/collections/CollectionUtils.java
> URL: http://svn.apache.org/viewvc/commons/proper/collections/branches/collections_jdk5_branch/src/java/org/apache/commons/collections/CollectionUtils.java?rev=593964&r1=593963&r2=593964&view=diff
> ==============================================================================
> --- commons/proper/collections/branches/collections_jdk5_branch/src/java/org/apache/commons/collections/CollectionUtils.java
(original)
> +++ commons/proper/collections/branches/collections_jdk5_branch/src/java/org/apache/commons/collections/CollectionUtils.java
Sun Nov 11 13:44:41 2007
> @@ -122,16 +122,14 @@
>  
>      }
>  
> -    /** 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<Object>());
> +    @SuppressWarnings("unchecked")
> +	public static final Collection EMPTY_COLLECTION = UnmodifiableCollection.decorate(new
ArrayList<Object>());
>  
>      /**
>       * <code>CollectionUtils</code> should not normally be instantiated.
> @@ -144,6 +142,7 @@
>       * 
>       * @see #EMPTY_COLLECTION
>       * @since 4.0
> +     * @return immutable empty collection
>       */
>      @SuppressWarnings("unchecked")
>      public static <T> Collection<T> emptyCollection() {
> @@ -162,8 +161,6 @@
>       * @param b the second collection, must not be null
>       * @param <O> the generic type that is able to represent the types contained
>       *        in both input collections.
> -     * @param <I1> the generic type of the first input {@link Iterable}.
> -     * @param <I2> the generic type of the second input {@link Iterable}.
>       * @return the union of the two collections
>       * @see Collection#addAll
>       */
> @@ -187,8 +184,6 @@
>       * @param b the second collection, must not be null
>       * @param <O> the generic type that is able to represent the types contained
>       *        in both input collections.
> -     * @param <I1> the generic type of the first input {@link Iterable}.
> -     * @param <I2> 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 @@
>       * @param b the second collection, must not be null
>       * @param <O> the generic type that is able to represent the types contained
>       *        in both input collections.
> -     * @param <I1> the generic type of the first input {@link Iterable}.
> -     * @param <I2> the generic type of the second input {@link Iterable}.
>       * @return the symmetric difference of the two collections
>       */
>      public static <O> Collection<O> disjunction(final Iterable<? extends
O> a, final Iterable<? extends O> b) {
> @@ -240,8 +233,6 @@
>       * @param b  the collection to subtract, must not be null
>       * @param <O> the generic type that is able to represent the types contained
>       *        in both input collections.
> -     * @param <I1> the generic type of the first input {@link Iterable}.
> -     * @param <I2> the generic type of the second input {@link Iterable}.
>       * @return a new collection with the results
>       * @see Collection#removeAll
>       */
> @@ -266,16 +257,16 @@
>       * @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 @@
>          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 static <T> void filter(Iterable<T> collection, Predicate<?
super T> predicate) {
>          if (collection != null && predicate != null) {
>              for (Iterator<T> it = collection.iterator(); it.hasNext();) {
> -                if (predicate.evaluate(it.next()) == false) {
> +                if (!predicate.evaluate(it.next())) {
>                      it.remove();
>                  }
>              }
> @@ -593,6 +584,7 @@
>       *            the predicate to use, may be null
>       * @param outputCollection
>       *            the collection to output into, may not be null
> +     * @return outputCollection
>       */
>      public static <O, I extends O> Collection<O> select(Collection<I>
inputCollection, Predicate<? super I> predicate, Collection<O> outputCollection)
{
>          if (inputCollection != null && predicate != null) {
> @@ -637,11 +629,12 @@
>       *            the predicate to use, may be null
>       * @param outputCollection
>       *            the collection to output into, may not be null
> +     * @return outputCollection
>       */
>      public static <O, I extends O> Collection<O> selectRejected(Collection<I>
inputCollection, Predicate<? super I> predicate, Collection<O> 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 @@
>       *            the transformer to use, may be null
>       * @param <I> the type of object in the input collection
>       * @param <O> the type of object in the output collection
> -     * @param <T> 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 @@
>       *            the transformer to use, may be null
>       * @param <I> the type of object in the input collection
>       * @param <O> the type of object in the output collection
> -     * @param <T> the output type of the transformer - this extends O.
>       * @return the transformed result (new list)
>       */
>      public static <I,O> Collection<O> collect(Iterator<I> inputIterator,
Transformer<? super I, ? extends O> transformer) {
> @@ -772,9 +763,9 @@
>       * @throws NullPointerException
>       *             if the collection or iterator is null
>       */
> -    public static <T> boolean addAll(Collection<? super T> collection, Iterable<T>
iterable) {
> +    public static <C> boolean addAll(Collection<C> collection, Iterable<?
extends C> iterable) {
>          if (iterable instanceof Collection) {
> -            return collection.addAll((Collection<T>) iterable);
> +            return collection.addAll((Collection<? extends C>) iterable);
>          }
>          return addAll(collection, iterable.iterator());
>      }
> @@ -843,15 +834,16 @@
>       * @throws IllegalArgumentException if the object type is invalid
>       */
>      public static <T> T get(Iterator<T> 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 @@
>       * @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 @@
>          }
>      }
>  
> -    private static final <T> int getFreq(final T obj, final Map<T, Integer>
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.
>       * <p>
> @@ -1246,7 +1230,7 @@
>       * @return a synchronized collection backed by the given collection
>       * @throws IllegalArgumentException  if the collection is null
>       */
> -    public static Collection synchronizedCollection(Collection collection) {
> +    public static <C> Collection<C> synchronizedCollection(Collection<C>
collection) {
>          return SynchronizedCollection.decorate(collection);
>      }
>  
> @@ -1259,7 +1243,7 @@
>       * @return an unmodifiable collection backed by the given collection
>       * @throws IllegalArgumentException  if the collection is null
>       */
> -    public static Collection unmodifiableCollection(Collection collection) {
> +    public static <C> Collection<C> unmodifiableCollection(Collection<C>
collection) {
>          return UnmodifiableCollection.decorate(collection);
>      }
>  
>
> Modified: commons/proper/collections/branches/collections_jdk5_branch/src/test/org/apache/commons/collections/TestCollectionUtils.java
> URL: http://svn.apache.org/viewvc/commons/proper/collections/branches/collections_jdk5_branch/src/test/org/apache/commons/collections/TestCollectionUtils.java?rev=593964&r1=593963&r2=593964&view=diff
> ==============================================================================
> --- commons/proper/collections/branches/collections_jdk5_branch/src/test/org/apache/commons/collections/TestCollectionUtils.java
(original)
> +++ commons/proper/collections/branches/collections_jdk5_branch/src/test/org/apache/commons/collections/TestCollectionUtils.java
Sun Nov 11 13:44:41 2007
> @@ -22,20 +22,7 @@
>  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 @@
>      private List<Long> 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<Integer> collectionC = null;
> @@ -93,7 +80,7 @@
>      private Collection<Number> 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<Number> collectionC2 = null;
> @@ -440,6 +427,16 @@
>      }
>  
>      @Test
> +    public void testIsEqualCollectionReturnsFalse() {
> +        List<Integer> b = new ArrayList<Integer>(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<String> a = new ArrayList<String>();
>          Collection<String> b = new ArrayList<String>();
> @@ -1073,7 +1070,7 @@
>              // expected
>          }
>          try {
> -            collection = CollectionUtils.predicatedCollection(null, predicate);
> +            CollectionUtils.predicatedCollection(null, predicate);
>              fail("Expecting IllegalArgumentException for null collection.");
>          } catch (IllegalArgumentException ex) {
>              // expected
> @@ -1109,6 +1106,12 @@
>      }
>  
>      @Test
> +    public void isEmpty() {
> +        assertFalse(CollectionUtils.isNotEmpty(null));
> +        assertTrue(CollectionUtils.isNotEmpty(collectionA));
> +    }
> +
> +    @Test
>      public void maxSize() {
>          Set<String> set = new HashSet<String>();
>          set.add("1");
> @@ -1346,6 +1349,68 @@
>          assertFalse(CollectionUtils.addAll(c, inputIterable));
>          verify();
>      }
> +
> +    @Test
> +    public void addAllForEnumeration() {
> +        Hashtable<Integer, Integer> h = new Hashtable();
> +        h.put(5, 5);
> +        Enumeration<? extends Integer> 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<Integer, Integer> 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
>
>
>   

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message