Skip to content

Commit

Permalink
[COLLECTIONS-580] Do not use InstantiateFactory anymore for MultiValu…
Browse files Browse the repository at this point in the history 
…edMaps: different MultiValuedMap implementations are now fully typed for the used underlying map and value collection class being used. This has pros and cons, but it is certainly safer to do it that way.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1715302 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@netomi
netomi committed Nov 19, 2015
1 parent ccda2db commit b2b8f4a
Show file tree
Hide file tree
Showing 24 changed files with 571 additions and 554 deletions.
37 changes: 5 additions & 32 deletions src/main/java/org/apache/commons/collections4/MultiMapUtils.java
View file
Expand Up @@ -23,7 +23,8 @@
import java.util.Set;

import org.apache.commons.collections4.bag.HashBag;
import org.apache.commons.collections4.multimap.MultiValuedHashMap;
import org.apache.commons.collections4.multimap.ArrayListValuedHashMap;
import org.apache.commons.collections4.multimap.HashSetValuedHashMap;
import org.apache.commons.collections4.multimap.TransformedMultiValuedMap;
import org.apache.commons.collections4.multimap.UnmodifiableMultiValuedMap;

Expand Down Expand Up @@ -52,7 +53,7 @@ private MultiMapUtils() {}
*/
@SuppressWarnings({ "rawtypes", "unchecked" })
public static final MultiValuedMap EMPTY_MULTI_VALUED_MAP =
UnmodifiableMultiValuedMap.unmodifiableMultiValuedMap(new MultiValuedHashMap());
UnmodifiableMultiValuedMap.unmodifiableMultiValuedMap(new ArrayListValuedHashMap(0, 0));

/**
* Returns immutable EMPTY_MULTI_VALUED_MAP with generic type safety.
Expand Down Expand Up @@ -188,21 +189,7 @@ public static <K, V> Bag<V> getValuesAsBag(final MultiValuedMap<K, V> map, final
* @return a new <code>ListValuedMap</code>
*/
public static <K, V> ListValuedMap<K, V> newListValuedHashMap() {
return MultiValuedHashMap.<K, V>listValuedHashMap();
}

/**
* Creates a {@link ListValuedMap} with a {@link java.util.HashMap HashMap} as its internal
* storage which maps the keys to list of type <code>listClass</code>.
*
* @param <K> the key type
* @param <V> the value type
* @param <C> the List class type
* @param listClass the class of the list
* @return a new {@link ListValuedMap}
*/
public static <K, V, C extends List<V>> ListValuedMap<K, V> newListValuedHashMap(final Class<C> listClass) {
return MultiValuedHashMap.<K, V, C>listValuedHashMap(listClass);
return new ArrayListValuedHashMap<K, V>();
}

/**
Expand All @@ -214,21 +201,7 @@ public static <K, V, C extends List<V>> ListValuedMap<K, V> newListValuedHashMap
* @return a new {@link SetValuedMap}
*/
public static <K, V> SetValuedMap<K, V> newSetValuedHashMap() {
return MultiValuedHashMap.<K, V>setValuedHashMap();
}

/**
* Creates a {@link SetValuedMap} with a {@link java.util.HashMap HashMap} as its internal
* storage which maps the keys to a set of type <code>setClass</code>
*
* @param <K> the key type
* @param <V> the value type
* @param <C> the Set class type
* @param setClass the class of the set
* @return a new {@link SetValuedMap}
*/
public static <K, V, C extends Set<V>> SetValuedMap<K, V> newSetValuedHashMap(final Class<C> setClass) {
return MultiValuedHashMap.<K, V, C>setValuedHashMap(setClass);
return new HashSetValuedHashMap<K, V>();
}

// MultiValuedMap Decorators
Expand Down
62 changes: 20 additions & 42 deletions src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java
View file
Expand Up @@ -38,45 +38,38 @@
public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap<K, V>
implements ListValuedMap<K, V> {

/** The serialization version */
private static final long serialVersionUID = 20150612L;

/**
* A constructor that wraps, not copies
*
* @param <C> the list type
* @param map the map to wrap, must not be null
* @param listClazz the collection class
* @throws NullPointerException if the map is null
* Constructor needed for subclass serialisation.
*/
protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz) {
super(map, listClazz);
protected AbstractListValuedMap() {
super();
}

/**
* A constructor that wraps, not copies
*
* @param <C> the list type
* @param map the map to wrap, must not be null
* @param listClazz the collection class
* @param initialListCapacity the initial size of the values list
* @throws NullPointerException if the map is null
* @throws IllegalArgumentException if initialListCapacity is negative
* @throws NullPointerException if the map is null
*/
protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz,
final int initialListCapacity) {
super(map, listClazz, initialListCapacity);
protected AbstractListValuedMap(final Map<K, ? extends List<V>> map) {
super(map);
}

// -----------------------------------------------------------------------
@Override
@SuppressWarnings("unchecked")
protected Map<K, List<V>> getMap() {
return (Map<K, List<V>>) super.getMap();
}

/**
* Creates a new value collection using the provided factory.
* @return a new list
*/
@Override
protected List<V> createCollection() {
return (List<V>) super.createCollection();
}
protected abstract List<V> createCollection();

// -----------------------------------------------------------------------
/**
* Gets the list of values associated with the specified key. This would
* return an empty list in case the mapping is not present
Expand All @@ -100,25 +93,10 @@ public List<V> get(final K key) {
*/
@Override
public List<V> remove(Object key) {
return ListUtils.emptyIfNull((List<V>) getMap().remove(key));
}

@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj instanceof ListValuedMap) {
return asMap().equals(((ListValuedMap<?, ?>) obj).asMap());
}
return false;
}

@Override
public int hashCode() {
return asMap().hashCode();
return ListUtils.emptyIfNull(getMap().remove(key));
}

// -----------------------------------------------------------------------
/**
* Wrapped list to handle add and remove on the list returned by get(object)
*/
Expand All @@ -130,7 +108,7 @@ public WrappedList(final K key) {

@Override
protected List<V> getMapping() {
return (List<V>) getMap().get(key);
return getMap().get(key);
}

@Override
Expand Down Expand Up @@ -237,13 +215,13 @@ private class ValuesListIterator implements ListIterator<V> {

public ValuesListIterator(final K key) {
this.key = key;
this.values = ListUtils.emptyIfNull((List<V>) getMap().get(key));
this.values = ListUtils.emptyIfNull(getMap().get(key));
this.iterator = values.listIterator();
}

public ValuesListIterator(final K key, int index) {
this.key = key;
this.values = ListUtils.emptyIfNull((List<V>) getMap().get(key));
this.values = ListUtils.emptyIfNull(getMap().get(key));
this.iterator = values.listIterator(index);
}

Expand Down
121 changes: 73 additions & 48 deletions src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java
View file
Expand Up @@ -16,7 +16,9 @@
*/
package org.apache.commons.collections4.multimap;

import java.io.Serializable;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.lang.reflect.Array;
import java.util.AbstractCollection;
import java.util.ArrayList;
Expand All @@ -27,13 +29,11 @@
import java.util.Set;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.Factory;
import org.apache.commons.collections4.IteratorUtils;
import org.apache.commons.collections4.MapIterator;
import org.apache.commons.collections4.MultiSet;
import org.apache.commons.collections4.MultiValuedMap;
import org.apache.commons.collections4.Transformer;
import org.apache.commons.collections4.functors.InstantiateFactory;
import org.apache.commons.collections4.iterators.EmptyMapIterator;
import org.apache.commons.collections4.iterators.IteratorChain;
import org.apache.commons.collections4.iterators.LazyIteratorChain;
Expand All @@ -50,13 +50,7 @@
* @since 4.1
* @version $Id$
*/
public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V>, Serializable {

/** Serialization Version */
private static final long serialVersionUID = 20150612L;

/** The factory for creating value collections. */
private final Factory<? extends Collection<V>> collectionFactory;
public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V> {

/** The values view */
private transient Collection<V> valuesView;
Expand All @@ -68,60 +62,55 @@ public abstract class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K,
private transient KeysMultiSet keysMultiSetView;

/** The map used to store the data */
private final Map<K, Collection<V>> map;
private transient Map<K, Collection<V>> map;

/**
* Constructor that wraps (not copies).
*
* @param <C> the collection type
* @param map the map to wrap, must not be null
* @param collectionClazz the collection class
* @throws NullPointerException if the map is null
* Constructor needed for subclass serialisation.
*/
@SuppressWarnings("unchecked")
protected <C extends Collection<V>> AbstractMultiValuedMap(final Map<K, ? super C> map,
final Class<C> collectionClazz) {
if (map == null) {
throw new NullPointerException("Map must not be null.");
}
this.map = (Map<K, Collection<V>>) map;
this.collectionFactory = new InstantiateFactory<C>(collectionClazz);
protected AbstractMultiValuedMap() {
super();
}

/**
* Constructor that wraps (not copies).
*
* @param <C> the collection type
* @param map the map to wrap, must not be null
* @param collectionClazz the collection class
* @param initialCollectionCapacity the initial capacity of the collection
* @throws NullPointerException if the map is null
* @throws IllegalArgumentException if initialCollectionCapacity is negative
* @throws NullPointerException if the map is null
*/
@SuppressWarnings("unchecked")
protected <C extends Collection<V>> AbstractMultiValuedMap(final Map<K, ? super C> map,
final Class<C> collectionClazz, final int initialCollectionCapacity) {
protected AbstractMultiValuedMap(final Map<K, ? extends Collection<V>> map) {
if (map == null) {
throw new NullPointerException("Map must not be null.");
}
if (initialCollectionCapacity < 0) {
throw new IllegalArgumentException("InitialCapacity must not be negative.");
}
this.map = (Map<K, Collection<V>>) map;
this.collectionFactory = new InstantiateFactory<C>(collectionClazz,
new Class[] { Integer.TYPE },
new Object[] { Integer.valueOf(initialCollectionCapacity) });
}

// -----------------------------------------------------------------------
/**
* Gets the map being wrapped.
*
* @return the wrapped map
*/
protected Map<K, Collection<V>> getMap() {
protected Map<K, ? extends Collection<V>> getMap() {
return map;
}

/**
* Sets the map being wrapped.
* <p>
* <b>NOTE:</b> this method should only be used during deserialization
*
* @param map the map to wrap
*/
@SuppressWarnings("unchecked")
protected void setMap(Map<K, ? extends Collection<V>> map) {
this.map = (Map<K, Collection<V>>) map;
}

protected abstract Collection<V> createCollection();

// -----------------------------------------------------------------------
@Override
public boolean containsKey(Object key) {
return getMap().containsKey(key);
Expand Down Expand Up @@ -250,7 +239,7 @@ public boolean put(final K key, final V value) {
if (coll == null) {
coll = createCollection();
if (coll.add(value)) {
getMap().put(key, coll);
map.put(key, coll);
return true;
} else {
return false;
Expand Down Expand Up @@ -325,8 +314,10 @@ public MultiSet<K> keys() {
}

@Override
@SuppressWarnings("unchecked")
public Map<K, Collection<V>> asMap() {
return getMap();
// TODO: return a view of the map
return (Map<K, Collection<V>>) getMap();
}

/**
Expand Down Expand Up @@ -373,18 +364,12 @@ public boolean equals(Object obj) {

@Override
public int hashCode() {
return getMap().hashCode();
return asMap().hashCode();
}

@Override
public String toString() {
return getMap().toString();
}

// -----------------------------------------------------------------------

protected Collection<V> createCollection() {
return collectionFactory.create();
return asMap().toString();
}

// -----------------------------------------------------------------------
Expand Down Expand Up @@ -906,4 +891,44 @@ public V next() {
}
}

//-----------------------------------------------------------------------
/**
* Write the map out using a custom routine.
* @param out the output stream
* @throws IOException any of the usual I/O related exceptions
*/
protected void doWriteObject(final ObjectOutputStream out) throws IOException {
out.writeInt(map.size());
for (final Map.Entry<K, Collection<V>> entry : map.entrySet()) {
out.writeObject(entry.getKey());
out.writeInt(entry.getValue().size());
for (final V value : entry.getValue()) {
out.writeObject(value);
}
}
}

/**
* Read the map in using a custom routine.
* @param in the input stream
* @throws IOException any of the usual I/O related exceptions
* @throws ClassNotFoundException if the stream contains an object which class can not be loaded
* @throws ClassCastException if the stream does not contain the correct objects
*/
protected void doReadObject(final ObjectInputStream in)
throws IOException, ClassNotFoundException {
final int entrySize = in.readInt();
for (int i = 0; i < entrySize; i++) {
@SuppressWarnings("unchecked") // This will fail at runtime if the stream is incorrect
final K key = (K) in.readObject();
final Collection<V> values = get(key);
final int valueSize = in.readInt();
for (int j = 0; j < valueSize; j++) {
@SuppressWarnings("unchecked") // see above
V value = (V) in.readObject();
values.add(value);
}
}
}

}

0 comments on commit b2b8f4a

Please sign in to comment.