First review of MultiValuedMap: formatting, throw NullPointerException for null inputs.

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/collections/trunk@1683013 13f79535-47bb-0310-9956-ffa450edef68

src/main/java/org/apache/commons/collections4/MultiMapUtils.java

@@ -30,10 +30,8 @@ import org.apache.commons.collections4.multimap.UnmodifiableMultiValuedMap;
 /**
  * Provides utility methods and decorators for {@link MultiValuedMap} instances.
  * <p>
- * It contains various type safe and null safe methods.
+ * It contains various type safe and null safe methods. Additionally, it provides
- * <p>
+ * the following decorators:
- * It also provides the following decorators:
- *
  * <ul>
  *   <li>{@link #unmodifiableMultiValuedMap(MultiValuedMap)}</li>
  *   <li>{@link #transformedMultiValuedMap(MultiValuedMap, Transformer, Transformer)}</li>
@@ -47,8 +45,7 @@ public class MultiMapUtils {
     /**
      * <code>MultiMapUtils</code> should not normally be instantiated.
      */
-    private MultiMapUtils() {
+    private MultiMapUtils() {}
-    }
 
     /**
      * An empty {@link UnmodifiableMultiValuedMap}.
@@ -77,8 +74,8 @@ public class MultiMapUtils {
      *
      * @param <K>  the type of key in the map
      * @param <V>  the type of value in the map
-     * @param map the map, possibly <code>null</code>
+     * @param map  the map, may be null
-     * @return an empty <code>MultiValuedMap</code> if the argument is <code>null</code>
+     * @return an empty {@link MultiValuedMap} if the argument is null
      */
     @SuppressWarnings("unchecked")
     public static <K, V> MultiValuedMap<K, V> emptyIfNull(final MultiValuedMap<K, V> map) {
@@ -88,10 +85,10 @@ public class MultiMapUtils {
     /**
      * Null-safe check if the specified <code>MultiValuedMap</code> is empty.
      * <p>
-     * Null returns true.
+     * If the provided map is null, returns true.
      *
      * @param map  the map to check, may be null
-     * @return true if empty or null
+     * @return true if the map is empty or null
      */
     public static boolean isEmpty(final MultiValuedMap<?, ?> map) {
         return map == null || map.isEmpty();
@@ -105,9 +102,9 @@ public class MultiMapUtils {
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to use
+     * @param map  the {@link MultiValuedMap} to use
      * @param key  the key to look up
-     * @return the Collection in the <code>MultiValuedMap</code>, <code>null</code> if map input is null
+     * @return the Collection in the {@link MultiValuedMap}, or null if input map is null
      */
     public static <K, V> Collection<V> getCollection(final MultiValuedMap<K, V> map, final K key) {
         if (map != null) {
@@ -116,17 +113,19 @@ public class MultiMapUtils {
         return null;
     }
 
+    // TODO: review the getValuesAsXXX methods - depending on the actual MultiValuedMap type, changes
+    // to the returned collection might update the backing map. This should be clarified and/or prevented.
+
     /**
      * Gets a List from <code>MultiValuedMap</code> in a null-safe manner.
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to use
+     * @param map  the {@link MultiValuedMap} to use
      * @param key  the key to look up
-     * @return the Collection in the <code>MultiValuedMap</code> as List,
+     * @return the Collection in the {@link MultiValuedMap} as List, or null if input map is null
-     *         <code>null</code> if map input is null
      */
-    public static <K, V> List<V> getList(MultiValuedMap<K, V> map, K key) {
+    public static <K, V> List<V> getValuesAsList(final MultiValuedMap<K, V> map, final K key) {
         if (map != null) {
             Collection<V> col = map.get(key);
             if (col instanceof List) {
@@ -142,12 +141,11 @@ public class MultiMapUtils {
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to use
+     * @param map  the {@link MultiValuedMap} to use
      * @param key  the key to look up
-     * @return the Collection in the <code>MultiValuedMap</code> as Set,
+     * @return the Collection in the {@link MultiValuedMap} as Set, or null if input map is null
-     *         <code>null</code> if map input is null
      */
-    public static <K, V> Set<V> getSet(MultiValuedMap<K, V> map, K key) {
+    public static <K, V> Set<V> getValuesAsSet(final MultiValuedMap<K, V> map, final K key) {
         if (map != null) {
             Collection<V> col = map.get(key);
             if (col instanceof Set) {
@@ -163,12 +161,11 @@ public class MultiMapUtils {
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to use
+     * @param map  the {@link MultiValuedMap} to use
      * @param key  the key to look up
-     * @return the Collection in the <code>MultiValuedMap</code> as Bag,
+     * @return the Collection in the {@link MultiValuedMap} as Bag, or null if input map is null
-     *         <code>null</code> if map input is null
      */
-    public static <K, V> Bag<V> getBag(MultiValuedMap<K, V> map, K key) {
+    public static <K, V> Bag<V> getValuesAsBag(final MultiValuedMap<K, V> map, final K key) {
         if (map != null) {
             Collection<V> col = map.get(key);
             if (col instanceof Bag) {
@@ -183,13 +180,14 @@ public class MultiMapUtils {
     // -----------------------------------------------------------------------
 
     /**
-     * Creates a {@link ListValuedMap} with a {@link java.util.HashMap HashMap} as its internal storage.
+     * Creates a {@link ListValuedMap} with an {@link java.util.ArrayList ArrayList} as
+     * collection class to store the values mapped to a key.
      *
      * @param <K>  the key type
      * @param <V>  the value type
      * @return a new <code>ListValuedMap</code>
      */
-    public static <K, V> ListValuedMap<K, V> createListValuedHashMap() {
+    public static <K, V> ListValuedMap<K, V> newListValuedHashMap() {
         return MultiValuedHashMap.<K, V>listValuedHashMap();
     }
 
@@ -201,21 +199,21 @@ public class MultiMapUtils {
      * @param <V>  the value type
      * @param <C>  the List class type
      * @param listClass the class of the list
-     * @return a new <code>ListValuedMap</code>
+     * @return a new {@link ListValuedMap}
      */
-    public static <K, V, C extends List<V>> ListValuedMap<K, V> createListValuedHashMap(final Class<C> listClass) {
+    public static <K, V, C extends List<V>> ListValuedMap<K, V> newListValuedHashMap(final Class<C> listClass) {
         return MultiValuedHashMap.<K, V, C>listValuedHashMap(listClass);
     }
 
     /**
-     * Creates a {@link SetValuedMap} with a {@link java.util.HashMap HashMap} as its internal
+     * Creates a {@link SetValuedMap} with an {@link java.util.HashSet HashSet} as
-     * storage
+     * collection class to store the values mapped to a key.
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @return a new <code>SetValuedMap</code>
+     * @return a new {@link SetValuedMap}
      */
-    public static <K, V> SetValuedMap<K, V> createSetValuedHashMap() {
+    public static <K, V> SetValuedMap<K, V> newSetValuedHashMap() {
         return MultiValuedHashMap.<K, V>setValuedHashMap();
     }
 
@@ -227,9 +225,9 @@ public class MultiMapUtils {
      * @param <V>  the value type
      * @param <C>  the Set class type
      * @param setClass  the class of the set
-     * @return a new <code>SetValuedMap</code>
+     * @return a new {@link SetValuedMap}
      */
-    public static <K, V, C extends Set<V>> SetValuedMap<K, V> createSetValuedHashMap(final Class<C> setClass) {
+    public static <K, V, C extends Set<V>> SetValuedMap<K, V> newSetValuedHashMap(final Class<C> setClass) {
         return MultiValuedHashMap.<K, V, C>setValuedHashMap(setClass);
     }
 
@@ -242,11 +240,9 @@ public class MultiMapUtils {
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to make unmodifiable, must not
+     * @param map  the {@link MultiValuedMap} to decorate, must not be null
-     *        be null
+     * @return an unmodifiable {@link MultiValuedMap} backed by the provided map
-     * @return an <code>UnmodifiableMultiValuedMap</code> backed by the given
+     * @throws IllegalArgumentException if map is null
-     *         map
-     * @throws IllegalArgumentException if the map is null
      */
     public static <K, V> MultiValuedMap<K, V> unmodifiableMultiValuedMap(
             final MultiValuedMap<? extends K, ? extends V> map) {
@@ -270,15 +266,11 @@ public class MultiMapUtils {
      *
      * @param <K>  the key type
      * @param <V>  the value type
-     * @param map the <code>MultiValuedMap</code> to transform, must not be
+     * @param map the {@link MultiValuedMap} to transform, must not be null, typically empty
-     *        null, typically empty
+     * @param keyTransformer the transformer for the map keys, null means no transformation
-     * @param keyTransformer the transformer for the map keys, null means no
+     * @param valueTransformer the transformer for the map values, null means no transformation
-     *        transformation
-     * @param valueTransformer the transformer for the map values, null means no
-     *        transformation
      * @return a transformed <code>MultiValuedMap</code> backed by the given map
-     * @throws IllegalArgumentException if the <code>MultiValuedMap</code> is
+     * @throws IllegalArgumentException if map is null
-     *         null
      */
     public static <K, V> MultiValuedMap<K, V> transformedMultiValuedMap(final MultiValuedMap<K, V> map,
             final Transformer<? super K, ? extends K> keyTransformer,

src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java

@@ -49,9 +49,9 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
      * @param <C> the list type
      * @param map the map to wrap, must not be null
      * @param listClazz the collection class
-     * @throws IllegalArgumentException if the map is null
+     * @throws NullPointerException if the map is null
      */
-    protected <C extends List<V>> AbstractListValuedMap(Map<K, ? super C> map, Class<C> listClazz) {
+    protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz) {
         super(map, listClazz);
     }
 
@@ -62,11 +62,11 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
      * @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 IllegalArgumentException if the map is null or if
+     * @throws NullPointerException if the map is null
-     *         initialListCapacity is negative
+     * @throws IllegalArgumentException if initialListCapacity is negative
      */
-    protected <C extends List<V>> AbstractListValuedMap(Map<K, ? super C> map, Class<C> listClazz,
+    protected <C extends List<V>> AbstractListValuedMap(final Map<K, ? super C> map, Class<C> listClazz,
-            int initialListCapacity) {
+                                                        final int initialListCapacity) {
         super(map, listClazz, initialListCapacity);
     }
 
@@ -75,8 +75,7 @@ public abstract class AbstractListValuedMap<K, V> extends AbstractMultiValuedMap
      * return an empty list in case the mapping is not present
      *
      * @param key the key to retrieve
-     * @return the <code>List</code> of values, will return an empty
+     * @return the <code>List</code> of values, will return an empty {@link List} for no mapping
-     *         <code>List</code> for no mapping
      * @throws ClassCastException if the key is of an invalid type
      */
     @Override

src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java

@@ -77,13 +77,13 @@ public class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V>, Seria
      * @param <C>  the collection type
      * @param map  the map to wrap, must not be null
      * @param collectionClazz  the collection class
-     * @throws IllegalArgumentException if the map is null
+     * @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) {
         if (map == null) {
-            throw new IllegalArgumentException("Map must not be null");
+            throw new NullPointerException("Map must not be null.");
         }
         this.map = (Map<K, Collection<V>>) map;
         this.collectionFactory = new InstantiateFactory<C>(collectionClazz);
@@ -96,14 +96,14 @@ public class AbstractMultiValuedMap<K, V> implements MultiValuedMap<K, V>, Seria
      * @param map the map to wrap, must not be null
      * @param collectionClazz the collection class
      * @param initialCollectionCapacity the initial capacity of the collection
-     * @throws IllegalArgumentException if the map is null or if
+     * @throws NullPointerException if the map is null
-     *         initialCollectionCapacity is negative
+     * @throws IllegalArgumentException if initialCollectionCapacity is negative
      */
     @SuppressWarnings("unchecked")
     protected <C extends Collection<V>> AbstractMultiValuedMap(final Map<K, ? super C> map,
             final Class<C> collectionClazz, final int initialCollectionCapacity) {
         if (map == null) {
-            throw new IllegalArgumentException("Map must not be null");
+            throw new NullPointerException("Map must not be null.");
         }
         if (initialCollectionCapacity < 0) {
             throw new IllegalArgumentException("Illegal Capacity: " + initialCollectionCapacity);

src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapDecorator.java

@@ -29,8 +29,9 @@ import org.apache.commons.collections4.MultiValuedMap;
 /**
  * Decorates another <code>MultiValuedMap</code> to provide additional behaviour.
  * <p>
- * Each method call made on this <code>MultiValuedMap</code> is forwarded to the decorated <code>MultiValuedMap</code>.
+ * Each method call made on this <code>MultiValuedMap</code> is forwarded to the
- * This class is used as a framework to build to extensions such as synchronized and unmodifiable behaviour.
+ * decorated <code>MultiValuedMap</code>. This class is used as a framework to build
+ * to extensions such as synchronized and unmodifiable behaviour.
  *
  * @param <K> the type of key elements
  * @param <V> the type of value elements
@@ -51,12 +52,11 @@ public class AbstractMultiValuedMapDecorator<K, V>
      * Constructor that wraps (not copies).
      *
      * @param map the map to decorate, must not be null
-     * @throws IllegalArgumentException if the map is null
+     * @throws NullPointerException if the map is null
      */
     protected AbstractMultiValuedMapDecorator(final MultiValuedMap<K, V> map) {
         if (map == null) {
-            throw new IllegalArgumentException(
+            throw new NullPointerException("MultiValuedMap must not be null.");
-                    "MultiValuedMap must not be null");
         }
         this.map = map;
     }

src/main/java/org/apache/commons/collections4/multimap/AbstractSetValuedMap.java

@@ -47,7 +47,7 @@ public abstract class AbstractSetValuedMap<K, V> extends AbstractMultiValuedMap<
      * @param <C> the set type
      * @param map the map to wrap, must not be null
      * @param setClazz the collection class
-     * @throws IllegalArgumentException if the map is null
+     * @throws NullPointerException if the map is null
      */
     protected <C extends Set<V>> AbstractSetValuedMap(Map<K, ? super C> map, Class<C> setClazz) {
         super(map, setClazz);
@@ -60,8 +60,8 @@ public abstract class AbstractSetValuedMap<K, V> extends AbstractMultiValuedMap<
      * @param map the map to wrap, must not be null
      * @param setClazz the collection class
      * @param initialSetCapacity the initial size of the values set
-     * @throws IllegalArgumentException if the map is null or if
+     * @throws NullPointerException if the map is null
-     *         initialSetCapacity is negative
+     * @throws IllegalArgumentException if initialSetCapacity is negative
      */
     protected <C extends Set<V>> AbstractSetValuedMap(Map<K, ? super C> map, Class<C> setClazz,
             int initialSetCapacity) {

src/main/java/org/apache/commons/collections4/multimap/MultiValuedHashMap.java

@@ -163,8 +163,7 @@ public class MultiValuedHashMap<K, V> extends AbstractMultiValuedMap<K, V> {
      *
      * @param initialCapacity the initial capacity of the underlying hash map
      * @param loadFactor the load factor of the underlying hash map
-     * @param initialCollectionCapacity the initial capacity of the Collection
+     * @param initialCollectionCapacity the initial capacity of the Collection of values
-     *        of values
      */
     @SuppressWarnings("unchecked")
     public MultiValuedHashMap(int initialCapacity, float loadFactor, int initialCollectionCapacity) {
@@ -211,17 +210,13 @@ public class MultiValuedHashMap<K, V> extends AbstractMultiValuedMap<K, V> {
 
     /**
      * Creates a MultiValuedHashMap which creates the value collections using
-     * the supplied <code>collectionClazz</code> and the initial collection
+     * the supplied <code>collectionClazz</code> and the initial collection capacity.
-     * capacity .
      *
-     * @param initialCapacity the initial capacity of the underlying
-     *        <code>HashMap</code>
-     * @param loadFactor the load factor of the underlying <code>HashMap</code>
-     * @param initialCollectionCapacity the initial capacity of the
-     *        <code>Collection</code>
      * @param <C> the collection type
-     * @param collectionClazz the class of the <code>Collection</code> to use to
+     * @param initialCapacity the initial capacity of the underlying <code>HashMap</code>
-     *        create the value collections
+     * @param loadFactor the load factor of the underlying <code>HashMap</code>
+     * @param initialCollectionCapacity the initial capacity of the <code>Collection</code>
+     * @param collectionClazz the class of the <code>Collection</code> to use to create the value collections
      */
     protected <C extends Collection<V>> MultiValuedHashMap(int initialCapacity, float loadFactor,
             final Class<C> collectionClazz, int initialCollectionCapacity) {

src/main/java/org/apache/commons/collections4/multimap/UnmodifiableMultiValuedMap.java

@@ -60,8 +60,8 @@ public class UnmodifiableMultiValuedMap<K, V>
      * @throws IllegalArgumentException if map is null
      */
     @SuppressWarnings("unchecked")
-    public static <K, V> UnmodifiableMultiValuedMap<K, V>
+    public static <K, V> UnmodifiableMultiValuedMap<K, V> unmodifiableMultiValuedMap(
-            unmodifiableMultiValuedMap(MultiValuedMap<? extends K, ? extends V> map) {
+            MultiValuedMap<? extends K, ? extends V> map) {
         if (map instanceof Unmodifiable) {
             return (UnmodifiableMultiValuedMap<K, V>) map;
         }

src/main/java/org/apache/commons/collections4/multimap/package-info.java

@@ -21,7 +21,6 @@
  * The following implementations are provided in the package:
  * <ul>
  *   <li>MultiValuedHashMap - implementation that uses a HashMap to store the data
- *   <li>MultiValuedLinkedHashMap - implementation that uses a LinkedHashMap as backing map
  * </ul>
  * <p>
  * The following decorators are provided in the package: