From 0f0b8710e24160576f29504ef3e3bcbe34f22770 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Sun, 17 Mar 2013 10:01:09 +0000 Subject: [PATCH] [COLLECTIONS-424] CompositeSet: remove inheritance from CompositeCollection, update mutator class and unit tests. git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1457410 13f79535-47bb-0310-9956-ffa450edef68 --- .../collection/CompositeCollection.java | 47 +- .../commons/collections/set/CompositeSet.java | 451 ++++++++++++++---- .../collection/CompositeCollectionTest.java | 8 +- .../collections/set/CompositeSetTest.java | 17 +- .../collections/set/EmptySetMutator.java | 12 +- 5 files changed, 386 insertions(+), 149 deletions(-) diff --git a/src/main/java/org/apache/commons/collections/collection/CompositeCollection.java b/src/main/java/org/apache/commons/collections/collection/CompositeCollection.java index 8388a305d..100d9d2eb 100644 --- a/src/main/java/org/apache/commons/collections/collection/CompositeCollection.java +++ b/src/main/java/org/apache/commons/collections/collection/CompositeCollection.java @@ -73,7 +73,8 @@ public class CompositeCollection implements Collection, Serializable { * @param compositeCollection1 the Collection to be appended to the composite * @param compositeCollection2 the Collection to be appended to the composite */ - public CompositeCollection(final Collection compositeCollection1, final Collection compositeCollection2) { + public CompositeCollection(final Collection compositeCollection1, + final Collection compositeCollection2) { super(); addComposited(compositeCollection1, compositeCollection2); } @@ -88,16 +89,6 @@ public class CompositeCollection implements Collection, Serializable { addComposited(compositeCollections); } -// /** -// * Create a Composite Collection extracting the collections from an iterable. -// * -// * @param compositeCollections the collections to composite -// */ -// public CompositeCollection(Iterable> compositeCollections) { -// super(); -// addComposited(compositeCollections); -// } - //----------------------------------------------------------------------- /** * Gets the size of this composite collection. @@ -122,7 +113,7 @@ public class CompositeCollection implements Collection, Serializable { * @return true if all of the contained collections are empty */ public boolean isEmpty() { - for (final Collection item : all) { + for (final Collection item : all) { if (item.isEmpty() == false) { return false; } @@ -139,7 +130,7 @@ public class CompositeCollection implements Collection, Serializable { * @return true if obj is contained in any of the contained collections */ public boolean contains(final Object obj) { - for (final Collection item : all) { + for (final Collection item : all) { if (item.contains(obj)) { return true; } @@ -162,7 +153,7 @@ public class CompositeCollection implements Collection, Serializable { return EmptyIterator.emptyIterator(); } final IteratorChain chain = new IteratorChain(); - for (final Collection item : all) { + for (final Collection item : all) { chain.addIterator(item.iterator()); } return chain; @@ -201,7 +192,7 @@ public class CompositeCollection implements Collection, Serializable { } int offset = 0; - for (final Collection item : all) { + for (final Collection item : all) { for (final E e : item) { result[offset++] = e; } @@ -303,7 +294,7 @@ public class CompositeCollection implements Collection, Serializable { return false; } boolean changed = false; - for (final Collection item : all) { + for (final Collection item : all) { changed |= item.removeAll(coll); } return changed; @@ -321,7 +312,7 @@ public class CompositeCollection implements Collection, Serializable { */ public boolean retainAll(final Collection coll) { boolean changed = false; - for (final Collection item : all) { + for (final Collection item : all) { changed |= item.retainAll(coll); } return changed; @@ -335,7 +326,7 @@ public class CompositeCollection implements Collection, Serializable { * @throws UnsupportedOperationException if clear is unsupported */ public void clear() { - for (final Collection coll : all) { + for (final Collection coll : all) { coll.clear(); } } @@ -365,7 +356,8 @@ public class CompositeCollection implements Collection, Serializable { * @param compositeCollection1 the Collection to be appended to the composite * @param compositeCollection2 the Collection to be appended to the composite */ - public void addComposited(final Collection compositeCollection1, final Collection compositeCollection2) { + public void addComposited(final Collection compositeCollection1, + final Collection compositeCollection2) { all.add(compositeCollection1); all.add(compositeCollection2); } @@ -379,17 +371,6 @@ public class CompositeCollection implements Collection, Serializable { all.addAll(Arrays.asList(compositeCollections)); } -// /** -// * Add these Collections to the list of collections in this composite -// * -// * @param compositeCollections the Collections to be appended to the composite -// */ -// public void addComposited(Iterable> compositeCollections) { -// for (Collection item : compositeCollections) { -// all.add(item); -// } -// } - /** * Removes a collection from the those being decorated in this composite. * @@ -415,7 +396,7 @@ public class CompositeCollection implements Collection, Serializable { * * @return Unmodifiable list of all collections in this composite. */ - public List> getCollections() { + public List> getCollections() { return UnmodifiableList.unmodifiableList(all); } @@ -477,7 +458,9 @@ public class CompositeCollection implements Collection, Serializable { * @throws NullPointerException if the object cannot be removed because its null * @throws IllegalArgumentException if the object cannot be removed */ - public boolean remove(CompositeCollection composite, List> collections, Object obj); + public boolean remove(CompositeCollection composite, + List> collections, + Object obj); } diff --git a/src/main/java/org/apache/commons/collections/set/CompositeSet.java b/src/main/java/org/apache/commons/collections/set/CompositeSet.java index b45e51fe9..3163bd318 100644 --- a/src/main/java/org/apache/commons/collections/set/CompositeSet.java +++ b/src/main/java/org/apache/commons/collections/set/CompositeSet.java @@ -16,12 +16,20 @@ */ package org.apache.commons.collections.set; +import java.io.Serializable; +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Set; import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.collections.collection.CompositeCollection; +import org.apache.commons.collections.iterators.EmptyIterator; +import org.apache.commons.collections.iterators.IteratorChain; +import org.apache.commons.collections.list.UnmodifiableList; /** * Decorates a set of other sets to provide a single unified view. @@ -29,17 +37,28 @@ import org.apache.commons.collections.collection.CompositeCollection; * Changes made to this set will actually be made on the decorated set. * Add operations require the use of a pluggable strategy. * If no strategy is provided then add is unsupported. - * + *

+ * From version 4.0, this class does not extend {@link CompositeCollection} + * anymore due to its input restrictions (only accepts Sets). + * See COLLECTIONS-424 + * for more details. + * * @since 3.0 * @version $Id$ */ -public class CompositeSet extends CompositeCollection implements Set { +public class CompositeSet implements Set, Serializable { /** Serialization version */ private static final long serialVersionUID = 5185069727540378940L; + /** SetMutator to handle changes to the collection */ + protected SetMutator mutator; + + /** Sets in the composite */ + protected List> all = new ArrayList>(); + /** - * Create an empty CompositeSet + * Create an empty CompositeSet. */ public CompositeSet() { super(); @@ -51,7 +70,8 @@ public class CompositeSet extends CompositeCollection implements Set { * @param set the initial set in the composite */ public CompositeSet(final Set set) { - super(set); + super(); + addComposited(set); } /** @@ -60,103 +80,143 @@ public class CompositeSet extends CompositeCollection implements Set { * @param sets the initial sets in the composite */ public CompositeSet(final Set... sets) { - super(sets); + super(); + addComposited(sets); + } + + //----------------------------------------------------------------------- + /** + * Gets the size of this composite set. + *

+ * This implementation calls size() on each set. + * + * @return total number of elements in all contained containers + */ + public int size() { + int size = 0; + for (final Set item : all) { + size += item.size(); + } + return size; } /** - * Add a Set to this composite + * Checks whether this composite set is empty. + *

+ * This implementation calls isEmpty() on each set. * - * @param c Must implement Set - * @throws IllegalArgumentException if c does not implement java.util.Set - * or if a SetMutator is set, but fails to resolve a collision - * @throws UnsupportedOperationException if there is no SetMutator set, or - * a CollectionMutator is set instead of a SetMutator - * @see org.apache.commons.collections.collection.CompositeCollection.CollectionMutator - * @see SetMutator + * @return true if all of the contained sets are empty */ - @Override - public synchronized void addComposited(final Collection c) { - if (!(c instanceof Set)) { - throw new IllegalArgumentException("Collections added must implement java.util.Set"); - } - - for (final Set set : getCollections()) { - final Collection intersects = CollectionUtils.intersection(set, c); - if (intersects.size() > 0) { - if (this.mutator == null) { - throw new UnsupportedOperationException( - "Collision adding composited collection with no SetMutator set"); - } - else if (!(this.mutator instanceof SetMutator)) { - throw new UnsupportedOperationException( - "Collision adding composited collection to a CompositeSet with a CollectionMutator " + - "instead of a SetMutator"); - } - getMutator().resolveCollision(this, set, (Set) c, intersects); - if (CollectionUtils.intersection(set, c).size() > 0) { - throw new IllegalArgumentException( - "Attempt to add illegal entry unresolved by SetMutator.resolveCollision()"); - } + public boolean isEmpty() { + for (final Set item : all) { + if (item.isEmpty() == false) { + return false; } } - super.addComposited(c); - } - - @SuppressWarnings("unchecked") - @Override - public List> getCollections() { - return (List>) super.getCollections(); + return true; } /** - * Add two sets to this composite. + * Checks whether this composite set contains the object. + *

+ * This implementation calls contains() on each set. * - * @param c the first {@link java.util.Set} to add to this composite - * @param d the second {@link java.util.Set} to add to this composite - * @throws IllegalArgumentException if c or d does not implement {@link java.util.Set} + * @param obj the object to search for + * @return true if obj is contained in any of the contained sets + */ + public boolean contains(final Object obj) { + for (final Set item : all) { + if (item.contains(obj)) { + return true; + } + } + return false; + } + + /** + * Gets an iterator over all the sets in this composite. + *

+ * This implementation uses an IteratorChain. + * + * @return an IteratorChain instance which supports + * remove(). Iteration occurs over contained collections in + * the order they were added, but this behavior should not be relied upon. + * @see IteratorChain + */ + public Iterator iterator() { + if (all.isEmpty()) { + return EmptyIterator.emptyIterator(); + } + final IteratorChain chain = new IteratorChain(); + for (final Set item : all) { + chain.addIterator(item.iterator()); + } + return chain; + } + + /** + * Returns an array containing all of the elements in this composite. + * + * @return an object array of all the elements in the collection + */ + public Object[] toArray() { + final Object[] result = new Object[size()]; + int i = 0; + for (final Iterator it = iterator(); it.hasNext(); i++) { + result[i] = it.next(); + } + return result; + } + + /** + * Returns an object array, populating the supplied array if possible. + * See Collection interface for full details. + * + * @param the type of the elements in the collection + * @param array the array to use, populating if possible + * @return an array of all the elements in the collection */ - @Override @SuppressWarnings("unchecked") - public synchronized void addComposited(final Collection c, final Collection d) { - if (!(c instanceof Set)) { - throw new IllegalArgumentException("Argument must implement java.util.Set"); + public T[] toArray(final T[] array) { + final int size = size(); + Object[] result = null; + if (array.length >= size) { + result = array; + } else { + result = (Object[]) Array.newInstance(array.getClass().getComponentType(), size); } - if (!(d instanceof Set)) { - throw new IllegalArgumentException("Argument must implement java.util.Set"); + + int offset = 0; + for (final Collection item : all) { + for (final E e : item) { + result[offset++] = e; + } } - this.addComposited(new Set[] { (Set) c, (Set) d }); + if (result.length > size) { + result[size] = null; + } + return (T[]) result; } /** - * Add an array of sets to this composite - * @param comps the {@link Collection} of {@link java.util.Set}s to add to this composite - * @throws IllegalArgumentException if any of the collections in comps does not implement {@link java.util.Set} + * Adds an object to the collection, throwing UnsupportedOperationException + * unless a SetMutator strategy is specified. + * + * @param obj the object to add + * @return {@code true} if the collection was modified + * @throws UnsupportedOperationException if SetMutator hasn't been set or add is unsupported + * @throws ClassCastException if the object cannot be added due to its type + * @throws NullPointerException if the object cannot be added because its null + * @throws IllegalArgumentException if the object cannot be added */ - @Override - public synchronized void addComposited(final Collection[] comps) { - for (int i = comps.length - 1; i >= 0; --i) { - this.addComposited(comps[i]); + public boolean add(final E obj) { + if (mutator == null) { + throw new UnsupportedOperationException( + "add() is not supported on CompositeSet without a SetMutator strategy"); } + return mutator.add(this, all, obj); } - /** - * This can receive either a - * {@link org.apache.commons.collections.collection.CompositeCollection.CollectionMutator CollectionMutator} - * or a {@link CompositeSet.SetMutator}. - * If a {@link org.apache.commons.collections.collection.CompositeCollection.CollectionMutator CollectionMutator} - * is used than conflicts when adding composited sets will throw IllegalArgumentException. - * - * @param mutator - * the {@link org.apache.commons.collections.collection.CompositeCollection.CollectionMutator CollectionMutator} - * to use for this composite - */ - @Override - public void setMutator(final CollectionMutator mutator) { - super.setMutator(mutator); - } - - /* Set operations */ - /** * If a CollectionMutator is defined for this CompositeSet then this * method will be called anyway. @@ -164,9 +224,8 @@ public class CompositeSet extends CompositeCollection implements Set { * @param obj object to be removed * @return true if the object is removed, false otherwise */ - @Override public boolean remove(final Object obj) { - for (final Set set : getCollections()) { + for (final Set set : getSets()) { if (set.contains(obj)) { return set.remove(obj); } @@ -174,6 +233,187 @@ public class CompositeSet extends CompositeCollection implements Set { return false; } + /** + * Checks whether this composite contains all the elements in the specified collection. + *

+ * This implementation calls contains() for each element in the + * specified collection. + * + * @param coll the collection to check for + * @return true if all elements contained + */ + public boolean containsAll(final Collection coll) { + for (final Object item : coll) { + if (contains(item) == false) { + return false; + } + } + return true; + } + + /** + * Adds a collection of elements to this composite, throwing + * UnsupportedOperationException unless a SetMutator strategy is specified. + * + * @param coll the collection to add + * @return true if the composite was modified + * @throws UnsupportedOperationException if SetMutator hasn't been set or add is unsupported + * @throws ClassCastException if the object cannot be added due to its type + * @throws NullPointerException if the object cannot be added because its null + * @throws IllegalArgumentException if the object cannot be added + */ + public boolean addAll(final Collection coll) { + if (mutator == null) { + throw new UnsupportedOperationException( + "addAll() is not supported on CompositeSet without a SetMutator strategy"); + } + return mutator.addAll(this, all, coll); + } + + /** + * Removes the elements in the specified collection from this composite set. + *

+ * This implementation calls removeAll on each collection. + * + * @param coll the collection to remove + * @return true if the composite was modified + * @throws UnsupportedOperationException if removeAll is unsupported + */ + public boolean removeAll(final Collection coll) { + if (coll.size() == 0) { + return false; + } + boolean changed = false; + for (final Collection item : all) { + changed |= item.removeAll(coll); + } + return changed; + } + + /** + * Retains all the elements in the specified collection in this composite set, + * removing all others. + *

+ * This implementation calls retainAll() on each collection. + * + * @param coll the collection to remove + * @return true if the composite was modified + * @throws UnsupportedOperationException if retainAll is unsupported + */ + public boolean retainAll(final Collection coll) { + boolean changed = false; + for (final Collection item : all) { + changed |= item.retainAll(coll); + } + return changed; + } + + /** + * Removes all of the elements from this composite set. + *

+ * This implementation calls clear() on each set. + * + * @throws UnsupportedOperationException if clear is unsupported + */ + public void clear() { + for (final Collection coll : all) { + coll.clear(); + } + } + + //----------------------------------------------------------------------- + /** + * Specify a SetMutator strategy instance to handle changes. + * + * @param mutator the mutator to use + */ + public void setMutator(final SetMutator mutator) { + this.mutator = mutator; + } + + /** + * Add a Set to this composite. + * + * @param set the set to add + * @throws IllegalArgumentException if a SetMutator is set, but fails to resolve a collision + * @throws UnsupportedOperationException if there is no SetMutator set + * @see SetMutator + */ + public synchronized void addComposited(final Set set) { + for (final Set existingSet : getSets()) { + final Collection intersects = CollectionUtils.intersection(existingSet, set); + if (intersects.size() > 0) { + if (this.mutator == null) { + throw new UnsupportedOperationException( + "Collision adding composited set with no SetMutator set"); + } + getMutator().resolveCollision(this, existingSet, set, intersects); + if (CollectionUtils.intersection(existingSet, set).size() > 0) { + throw new IllegalArgumentException( + "Attempt to add illegal entry unresolved by SetMutator.resolveCollision()"); + } + } + } + all.add(set); + } + + /** + * Add these Sets to the list of sets in this composite. + * + * @param set1 the first Set to be appended to the composite + * @param set2 the second Set to be appended to the composite + */ + public void addComposited(final Set set1, final Set set2) { + all.add(set1); + all.add(set2); + } + + /** + * Add these Sets to the list of sets in this composite + * + * @param sets the Sets to be appended to the composite + */ + public void addComposited(final Set[] sets) { + all.addAll(Arrays.asList(sets)); + } + + /** + * Removes a set from those being decorated in this composite. + * + * @param set set to be removed + */ + public void removeComposited(final Set set) { + all.remove(set); + } + + //----------------------------------------------------------------------- + /** + * Returns a new Set containing all of the elements. + * + * @return A new HashSet containing all of the elements in this composite. + * The new collection is not backed by this composite. + */ + public Set toSet() { + return new HashSet(this); + } + + /** + * Gets the sets being decorated. + * + * @return Unmodifiable list of all sets in this composite. + */ + public List> getSets() { + return UnmodifiableList.unmodifiableList(all); + } + + /** + * Get the set mutator to be used for this CompositeSet. + * @return the set mutator + */ + protected SetMutator getMutator() { + return mutator; + } + /** * {@inheritDoc} * @see java.util.Set#equals @@ -182,7 +422,7 @@ public class CompositeSet extends CompositeCollection implements Set { public boolean equals(final Object obj) { if (obj instanceof Set) { final Set set = (Set) obj; - return set.containsAll(this) && set.size() == this.size(); + return set.size() == this.size() && set.containsAll(this); } return false; } @@ -200,18 +440,40 @@ public class CompositeSet extends CompositeCollection implements Set { return code; } - @Override - protected SetMutator getMutator() { - return (SetMutator) super.getMutator(); - } - /** * Define callbacks for mutation operations. - *

- * Defining remove() on implementations of SetMutator is pointless - * as they are never called by CompositeSet. */ - public static interface SetMutator extends CompositeCollection.CollectionMutator { + public static interface SetMutator extends Serializable { + + /** + * Called when an object is to be added to the composite. + * + * @param composite the CompositeSet being changed + * @param sets all of the Set instances in this CompositeSet + * @param obj the object being added + * @return true if the collection is changed + * @throws UnsupportedOperationException if add is unsupported + * @throws ClassCastException if the object cannot be added due to its type + * @throws NullPointerException if the object cannot be added because its null + * @throws IllegalArgumentException if the object cannot be added + */ + public boolean add(CompositeSet composite, List> sets, E obj); + + /** + * Called when a collection is to be added to the composite. + * + * @param composite the CompositeSet being changed + * @param sets all of the Set instances in this CompositeSet + * @param coll the collection being added + * @return true if the collection is changed + * @throws UnsupportedOperationException if add is unsupported + * @throws ClassCastException if the object cannot be added due to its type + * @throws NullPointerException if the object cannot be added because its null + * @throws IllegalArgumentException if the object cannot be added + */ + public boolean addAll(CompositeSet composite, + List> sets, + Collection coll); /** * Called when a Set is added to the CompositeSet and there is a @@ -225,6 +487,9 @@ public class CompositeSet extends CompositeCollection implements Set { * @param added the Set being added to the composite * @param intersects the intersection of the existing and added sets */ - public void resolveCollision(CompositeSet comp, Set existing, Set added, Collection intersects); + public void resolveCollision(CompositeSet comp, + Set existing, + Set added, + Collection intersects); } } diff --git a/src/test/java/org/apache/commons/collections/collection/CompositeCollectionTest.java b/src/test/java/org/apache/commons/collections/collection/CompositeCollectionTest.java index e8673d4ea..2067fa253 100644 --- a/src/test/java/org/apache/commons/collections/collection/CompositeCollectionTest.java +++ b/src/test/java/org/apache/commons/collections/collection/CompositeCollectionTest.java @@ -126,10 +126,10 @@ public class CompositeCollectionTest extends AbstractCollectionTest { protected void setUpMutatorTest() { setUpTest(); c.setMutator(new CompositeCollection.CollectionMutator() { - public boolean add(final CompositeCollection composite, - final List> collections, final E obj) { - for (final Collection collection : collections) { - collection.add(obj); + + public boolean add(CompositeCollection composite, List> collections, E obj) { + for (final Collection coll : collections) { + coll.add(obj); } return true; } diff --git a/src/test/java/org/apache/commons/collections/set/CompositeSetTest.java b/src/test/java/org/apache/commons/collections/set/CompositeSetTest.java index 1b2d47522..67ad40ed1 100644 --- a/src/test/java/org/apache/commons/collections/set/CompositeSetTest.java +++ b/src/test/java/org/apache/commons/collections/set/CompositeSetTest.java @@ -21,7 +21,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import org.apache.commons.collections.collection.CompositeCollection; +import org.apache.commons.collections.set.CompositeSet.SetMutator; /** * Extension of {@link AbstractSetTest} for exercising the @@ -94,7 +94,7 @@ public class CompositeSetTest extends AbstractSetTest { final Set one = buildOne(); final Set two = buildTwo(); final CompositeSet set = new CompositeSet(new Set[] { one, two }); - set.setMutator(new CompositeSet.SetMutator() { + set.setMutator(new SetMutator() { private static final long serialVersionUID = 1L; public void resolveCollision(final CompositeSet comp, final Set existing, @@ -102,18 +102,13 @@ public class CompositeSetTest extends AbstractSetTest { //noop } - public boolean add(final CompositeCollection composite, - final List> collections, final E obj) { + public boolean add(final CompositeSet composite, + final List> collections, final E obj) { throw new UnsupportedOperationException(); } - public boolean addAll(final CompositeCollection composite, - final List> collections, final Collection coll) { - throw new UnsupportedOperationException(); - } - - public boolean remove(final CompositeCollection composite, - final List> collections, final Object obj) { + public boolean addAll(final CompositeSet composite, + final List> collections, final Collection coll) { throw new UnsupportedOperationException(); } }); diff --git a/src/test/java/org/apache/commons/collections/set/EmptySetMutator.java b/src/test/java/org/apache/commons/collections/set/EmptySetMutator.java index a4e14e2f5..9f9840e40 100644 --- a/src/test/java/org/apache/commons/collections/set/EmptySetMutator.java +++ b/src/test/java/org/apache/commons/collections/set/EmptySetMutator.java @@ -20,8 +20,6 @@ import java.util.Collection; import java.util.List; import java.util.Set; -import org.apache.commons.collections.collection.CompositeCollection; - /** * This class is used in CompositeSetTest. When testing serialization, * the class has to be separate of CompositeSetTest, else the test @@ -42,15 +40,11 @@ class EmptySetMutator implements CompositeSet.SetMutator { throw new IllegalArgumentException(); } - public boolean add(final CompositeCollection composite, final List> collections, final E obj) { + public boolean add(final CompositeSet composite, final List> collections, final E obj) { return contained.add(obj); } - public boolean addAll(final CompositeCollection composite, final List> collections, final Collection coll) { + public boolean addAll(final CompositeSet composite, final List> collections, final Collection coll) { return contained.addAll(coll); - } - - public boolean remove(final CompositeCollection composite, final List> collections, final Object obj) { - return contained.remove(obj); - } + } }