diff --git a/src/main/java/org/apache/commons/collections4/FluentIterable.java b/src/main/java/org/apache/commons/collections4/FluentIterable.java index c4e0b2824..af99db39f 100644 --- a/src/main/java/org/apache/commons/collections4/FluentIterable.java +++ b/src/main/java/org/apache/commons/collections4/FluentIterable.java @@ -117,15 +117,13 @@ public class FluentIterable implements Iterable { * corresponding input iterator supports it. * * @param the element type - * @param iterable the iterable to wrap into a FluentIterable, may be null + * @param iterable the iterable to wrap into a FluentIterable, may not be null * @return a new FluentIterable wrapping the provided iterable + * @throws NullPointerException if iterable is null */ public static FluentIterable of(final Iterable iterable) { - if (iterable == null) { - @SuppressWarnings("unchecked") - final FluentIterable empty = IterableUtils.EMPTY_ITERABLE; - return empty; - } else if (iterable instanceof FluentIterable) { + IterableUtils.checkNotNull(iterable); + if (iterable instanceof FluentIterable) { return (FluentIterable) iterable; } else { return new FluentIterable(iterable); @@ -169,11 +167,10 @@ public class FluentIterable implements Iterable { * Returns a new FluentIterable whose iterator will first traverse * the elements of the current iterable, followed by the elements * of the provided iterable. - *

- * A null iterable will be treated as an empty iterable. * - * @param other the other iterable to combine, may be null + * @param other the other iterable to combine, may not be null * @return a new iterable, combining this iterable with other + * @throws NullPointerException if other is null */ public FluentIterable append(final Iterable other) { return of(IterableUtils.chainedIterable(iterable, other)); @@ -191,11 +188,10 @@ public class FluentIterable implements Iterable { *

* The returned iterable will traverse the elements in the following * order: [1, 2, 3, 4, 5, 6, 7, 8] - *

- * A null iterable will be treated as an empty iterable. * - * @param other the other iterable to collate, may be null + * @param other the other iterable to collate, may not be null * @return a new iterable, collating this iterable with the other in natural order + * @throws NullPointerException if other is null * @see {@link org.apache.commons.collections4.iterators.CollatingIterator CollatingIterator} */ public FluentIterable collate(final Iterable other) { @@ -215,13 +211,12 @@ public class FluentIterable implements Iterable { *

* The returned iterable will traverse the elements in the following * order: [8, 7, 6, 5, 4, 3, 2, 1] - *

- * A null iterable will be treated as an empty iterable. * * @param comparator the comparator to define an ordering, may be null, * in which case natural ordering will be used - * @param other the other iterable to collate, may be null + * @param other the other iterable to collate, may not be null * @return a new iterable, collating this iterable with the other in natural order + * @throws NullPointerException if other is null * @see {@link org.apache.commons.collections4.iterators.CollatingIterator CollatingIterator} */ public FluentIterable collate(final Iterable other, @@ -341,8 +336,9 @@ public class FluentIterable implements Iterable { * the elements of this iterable and the other iterable in * alternating order. * - * @param other the other iterable to interleave + * @param other the other iterable to interleave, may not be null * @return a new iterable, interleaving this iterable with others + * @throws NullPointerException if other is null */ public FluentIterable zip(final Iterable other) { return of(IterableUtils.zippingIterable(iterable, other)); @@ -353,15 +349,12 @@ public class FluentIterable implements Iterable { * the elements of this iterable and the other iterables in * alternating order. * - * @param others the iterables to interleave + * @param others the iterables to interleave, may not be null * @return a new iterable, interleaving this iterable with others + * @throws NullPointerException if either of the provided iterables is null */ public FluentIterable zip(final Iterable... others) { - @SuppressWarnings("unchecked") - Iterable[] iterables = new Iterable[1 + others.length]; - iterables[0] = iterable; - System.arraycopy(others, 0, iterables, 1, others.length); - return of(IterableUtils.zippingIterable(iterables)); + return of(IterableUtils.zippingIterable(iterable, others)); } // convenience methods diff --git a/src/main/java/org/apache/commons/collections4/IterableUtils.java b/src/main/java/org/apache/commons/collections4/IterableUtils.java index 8a3acddde..89983e630 100644 --- a/src/main/java/org/apache/commons/collections4/IterableUtils.java +++ b/src/main/java/org/apache/commons/collections4/IterableUtils.java @@ -32,10 +32,17 @@ import org.apache.commons.collections4.iterators.UniqueFilterIterator; /** * Provides utility methods and decorators for {@link Iterable} instances. *

- * Note: by design, all provided utility methods will treat a {@code null} - * {@link Iterable} parameters the same way as an empty iterable. All other required - * parameters which are null, e.g. a {@link Predicate}, will result in a - * {@link NullPointerException}. + * Note: this util class has been designed for fail-fast argument checking. + *

    + *
  • + * all decorator methods are NOT null-safe wrt the provided Iterable argument, i.e. + * they will throw a {@link NullPointerException} if a null Iterable is passed as argument. + *
  • + * all other utility methods are null-safe wrt the provided Iterable argument, i.e. they will + * treat a null Iterable the same way as an empty one. Other arguments which are null, + * e.g. a {@link Predicate}, will result in a {@link NullPointerException}. Exception: passing + * a null {@link Comparator} is equivalent to a Comparator with natural ordering. + *
* * @since 4.1 * @version $Id$ @@ -83,9 +90,10 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param a the first iterable - * @param b the second iterable + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null * @return a new iterable, combining the provided iterables + * @throws NullPointerException if either a or b is null */ @SuppressWarnings("unchecked") public static Iterable chainedIterable(final Iterable a, @@ -104,10 +112,11 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param a the first iterable - * @param b the second iterable - * @param c the third iterable + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null + * @param c the third iterable, may not be null * @return a new iterable, combining the provided iterables + * @throws NullPointerException if either of the provided iterables is null */ @SuppressWarnings("unchecked") public static Iterable chainedIterable(final Iterable a, @@ -127,11 +136,12 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param a the first iterable - * @param b the second iterable - * @param c the third iterable - * @param d the fourth iterable + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null + * @param c the third iterable, may not be null + * @param d the fourth iterable, may not be null * @return a new iterable, combining the provided iterables + * @throws NullPointerException if either of the provided iterables is null */ @SuppressWarnings("unchecked") public static Iterable chainedIterable(final Iterable a, @@ -152,10 +162,12 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param iterables the iterables to combine + * @param iterables the iterables to combine, may not be null * @return a new iterable, combining the provided iterables + * @throws NullPointerException if either of the provided iterables is null */ public static Iterable chainedIterable(final Iterable... iterables) { + checkNotNull(iterables); return new FluentIterable() { @Override public Iterator iterator() { @@ -165,7 +177,7 @@ public class IterableUtils { if (count > iterables.length) { return null; } else { - return emptyIteratorIfNull(iterables[count - 1]); + return iterables[count - 1].iterator(); } } }; @@ -184,18 +196,18 @@ public class IterableUtils { * corresponding input iterator supports it. * * @param the element type - * @param a the first iterable, may be null - * @param b the second iterable, may be null + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null * @return a filtered view on the specified iterable + * @throws NullPointerException if either of the provided iterables is null */ public static Iterable collatedIterable(final Iterable a, final Iterable b) { + checkNotNull(a, b); return new FluentIterable() { @Override public Iterator iterator() { - return IteratorUtils.collatedIterator(null, - emptyIteratorIfNull(a), - emptyIteratorIfNull(b)); + return IteratorUtils.collatedIterator(null, a.iterator(), b.iterator()); } }; } @@ -211,19 +223,19 @@ public class IterableUtils { * @param the element type * @param comparator the comparator defining an ordering over the elements, * may be null, in which case natural ordering will be used - * @param a the first iterable, may be null - * @param b the second iterable, may be null + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null * @return a filtered view on the specified iterable + * @throws NullPointerException if either of the provided iterables is null */ public static Iterable collatedIterable(final Comparator comparator, final Iterable a, final Iterable b) { + checkNotNull(a, b); return new FluentIterable() { @Override public Iterator iterator() { - return IteratorUtils.collatedIterator(comparator, - emptyIteratorIfNull(a), - emptyIteratorIfNull(b)); + return IteratorUtils.collatedIterator(comparator, a.iterator(), b.iterator()); } }; } @@ -238,17 +250,17 @@ public class IterableUtils { * The returned iterable's iterator does not support {@code remove()}. * * @param the element type - * @param iterable the iterable to filter, may be null + * @param iterable the iterable to filter, may not be null * @param predicate the predicate used to filter elements, may not be null * @return a filtered view on the specified iterable - * @throws NullPointerException if predicate is null + * @throws NullPointerException if either iterable or predicate is null */ public static Iterable filteredIterable(final Iterable iterable, final Predicate predicate) { + checkNotNull(iterable); if (predicate == null) { throw new NullPointerException("Predicate must not be null."); } - return new FluentIterable() { @Override public Iterator iterator() { @@ -268,12 +280,14 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param iterable the iterable to limit, may be null + * @param iterable the iterable to limit, may not be null * @param maxSize the maximum number of elements, must not be negative * @return a bounded view on the specified iterable * @throws IllegalArgumentException if maxSize is negative + * @throws NullPointerException if iterable is null */ public static Iterable boundedIterable(final Iterable iterable, final long maxSize) { + checkNotNull(iterable); if (maxSize < 0) { throw new IllegalArgumentException("MaxSize parameter must not be negative."); } @@ -281,7 +295,7 @@ public class IterableUtils { return new FluentIterable() { @Override public Iterator iterator() { - return IteratorUtils.boundedIterator(emptyIteratorIfNull(iterable), maxSize); + return IteratorUtils.boundedIterator(iterable.iterator(), maxSize); } }; } @@ -300,24 +314,22 @@ public class IterableUtils { * is empty. * * @param the element type - * @param iterable the iterable to loop, may be null + * @param iterable the iterable to loop, may not be null * @return a view of the iterable, providing an infinite loop over its elements + * @throws NullPointerException if iterable is null */ public static Iterable loopingIterable(final Iterable iterable) { + checkNotNull(iterable); return new FluentIterable() { @Override public Iterator iterator() { return new LazyIteratorChain() { @Override protected Iterator nextIterator(int count) { - if (iterable != null) { - if (IterableUtils.isEmpty(iterable)) { - return null; - } else { - return iterable.iterator(); - } - } else { + if (IterableUtils.isEmpty(iterable)) { return null; + } else { + return iterable.iterator(); } } }; @@ -339,18 +351,19 @@ public class IterableUtils { * provided iterable is a {@link List} instance. * * @param the element type - * @param iterable the iterable to use, may be null + * @param iterable the iterable to use, may not be null * @return a reversed view of the specified iterable + * @throws NullPointerException if iterable is null * @see ReverseListIterator */ public static Iterable reversedIterable(final Iterable iterable) { + checkNotNull(iterable); return new FluentIterable() { @Override public Iterator iterator() { final List list = (iterable instanceof List) ? (List) iterable : - IteratorUtils.toList(emptyIteratorIfNull(iterable)); - + IteratorUtils.toList(iterable.iterator()); return new ReverseListIterator(list); } }; @@ -366,12 +379,14 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param iterable the iterable to use, may be null + * @param iterable the iterable to use, may not be null * @param elementsToSkip the number of elements to skip from the start, must not be negative * @return a view of the specified iterable, skipping the first N elements * @throws IllegalArgumentException if elementsToSkip is negative + * @throws NullPointerException if iterable is null */ public static Iterable skippingIterable(final Iterable iterable, final long elementsToSkip) { + checkNotNull(iterable); if (elementsToSkip < 0) { throw new IllegalArgumentException("ElementsToSkip parameter must not be negative."); } @@ -379,7 +394,7 @@ public class IterableUtils { return new FluentIterable() { @Override public Iterator iterator() { - return IteratorUtils.skippingIterator(emptyIteratorIfNull(iterable), elementsToSkip); + return IteratorUtils.skippingIterator(iterable.iterator(), elementsToSkip); } }; } @@ -396,21 +411,21 @@ public class IterableUtils { * * @param the input element type * @param the output element type - * @param iterable the iterable to transform, may be null + * @param iterable the iterable to transform, may not be null * @param transformer the transformer, must not be null * @return a transformed view of the specified iterable - * @throws NullPointerException if transformer is null + * @throws NullPointerException if either iterable or transformer is null */ public static Iterable transformedIterable(final Iterable iterable, final Transformer transformer) { + checkNotNull(iterable); if (transformer == null) { throw new NullPointerException("Transformer must not be null."); } - return new FluentIterable() { @Override public Iterator iterator() { - return IteratorUtils.transformedIterator(emptyIteratorIfNull(iterable), transformer); + return IteratorUtils.transformedIterator(iterable.iterator(), transformer); } }; } @@ -424,14 +439,16 @@ public class IterableUtils { * The returned iterable's iterator does not support {@code remove()}. * * @param the element type - * @param iterable the iterable to use, may be null + * @param iterable the iterable to use, may not be null * @return a unique view of the specified iterable + * @throws NullPointerException if iterable is null */ public static Iterable uniqueIterable(final Iterable iterable) { + checkNotNull(iterable); return new FluentIterable() { @Override public Iterator iterator() { - return new UniqueFilterIterator(emptyIteratorIfNull(iterable)); + return new UniqueFilterIterator(iterable.iterator()); } }; } @@ -445,14 +462,16 @@ public class IterableUtils { * The returned iterable's iterator does not support {@code remove()}. * * @param the element type - * @param iterable the iterable to use, may be null + * @param iterable the iterable to use, may not be null * @return an unmodifiable view of the specified iterable + * @throws NullPointerException if iterable is null */ public static Iterable unmodifiableIterable(final Iterable iterable) { + checkNotNull(iterable); if (iterable instanceof UnmodifiableIterable) { return iterable; } - return new UnmodifiableIterable(emptyIfNull(iterable)); + return new UnmodifiableIterable(iterable); } /** @@ -486,14 +505,21 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param a the first iterable - * @param b the second iterable + * @param a the first iterable, may not be null + * @param b the second iterable, may not be null * @return a new iterable, interleaving the provided iterables + * @throws NullPointerException if either a or b is null */ - @SuppressWarnings("unchecked") public static Iterable zippingIterable(final Iterable a, final Iterable b) { - return zippingIterable(new Iterable[] {a, b}); + checkNotNull(a); + checkNotNull(b); + return new FluentIterable() { + @Override + public Iterator iterator() { + return IteratorUtils.zippingIterator(a.iterator(), b.iterator()); + } + }; } /** @@ -507,17 +533,22 @@ public class IterableUtils { * input iterator supports it. * * @param the element type - * @param iterables the array of iterables to interleave + * @param iterables the array of iterables to interleave, may not be null * @return a new iterable, interleaving the provided iterables + * @throws NullPointerException if either of the provided iterables is null */ - public static Iterable zippingIterable(final Iterable... iterables) { + public static Iterable zippingIterable(final Iterable first, + final Iterable... others) { + checkNotNull(first); + checkNotNull(others); return new FluentIterable() { @Override public Iterator iterator() { - @SuppressWarnings("unchecked") - Iterator[] iterators = new Iterator[iterables.length]; - for (int i = 0; i < iterables.length; i++) { - iterators[i] = emptyIteratorIfNull(iterables[i]); + @SuppressWarnings("unchecked") // safe + Iterator[] iterators = new Iterator[others.length + 1]; + iterators[0] = first.iterator(); + for (int i = 0; i < others.length; i++) { + iterators[i + 1] = others[i].iterator(); } return IteratorUtils.zippingIterator(iterators); } @@ -539,18 +570,6 @@ public class IterableUtils { return iterable == null ? IterableUtils.emptyIterable() : iterable; } - /** - * Returns an empty iterator if the argument is null, - * or {@code iterable.iterator()} otherwise. - * - * @param the element type - * @param iterable the iterable, possibly null - * @return an empty iterator if the argument is null - */ - private static Iterator emptyIteratorIfNull(final Iterable iterable) { - return iterable != null ? iterable.iterator() : IteratorUtils.emptyIterator(); - } - /** * Applies the closure to each element of the provided iterable. * @@ -639,7 +658,7 @@ public class IterableUtils { if (predicate == null) { throw new NullPointerException("Predicate must not be null."); } - return size(filteredIterable(input, predicate)); + return size(filteredIterable(emptyIfNull(input), predicate)); } /** @@ -716,7 +735,7 @@ public class IterableUtils { if (iterable instanceof Bag) { return ((Bag) iterable).getCount(obj); } - return size(filteredIterable(iterable, EqualPredicate.equalPredicate(obj))); + return size(filteredIterable(emptyIfNull(iterable), EqualPredicate.equalPredicate(obj))); } /** @@ -971,7 +990,7 @@ public class IterableUtils { public static String toString(final Iterable iterable, final Transformer transformer) { if (transformer == null) { - throw new NullPointerException("transformer may not be null"); + throw new NullPointerException("Transformer must not be null."); } return IteratorUtils.toString(emptyIteratorIfNull(iterable), transformer); } @@ -1002,4 +1021,46 @@ public class IterableUtils { transformer, delimiter, prefix, suffix); } + // Helper methods + // ---------------------------------------------------------------------- + + /** + * Fail-fast check for null arguments. + * + * @param iterable the iterable to check + * @throws NullPointerException if iterable is null + */ + static void checkNotNull(final Iterable iterable) { + if (iterable == null) { + throw new NullPointerException("Iterable must not be null."); + } + } + + /** + * Fail-fast check for null arguments. + * + * @param iterable the iterable to check + * @throws NullPointerException if the argument or any of its contents is null + */ + static void checkNotNull(final Iterable... iterables) { + if (iterables == null) { + throw new NullPointerException("Iterables must not be null."); + } + for (final Iterable iterable : iterables) { + checkNotNull(iterable); + } + } + + /** + * Returns an empty iterator if the argument is null, + * or {@code iterable.iterator()} otherwise. + * + * @param the element type + * @param iterable the iterable, possibly null + * @return an empty iterator if the argument is null + */ + private static Iterator emptyIteratorIfNull(final Iterable iterable) { + return iterable != null ? iterable.iterator() : IteratorUtils.emptyIterator(); + } + } diff --git a/src/test/java/org/apache/commons/collections4/FluentIterableTest.java b/src/test/java/org/apache/commons/collections4/FluentIterableTest.java index ac7c38e1e..f687515a0 100644 --- a/src/test/java/org/apache/commons/collections4/FluentIterableTest.java +++ b/src/test/java/org/apache/commons/collections4/FluentIterableTest.java @@ -105,15 +105,25 @@ public class FluentIterableTest { // ----------------------------------------------------------------------- @Test public void factoryMethodOf() { - List result = FluentIterable.of(1, 2, 3, 4, 5).toList(); + FluentIterable iterable = FluentIterable.of(1, 2, 3, 4, 5); + List result = iterable.toList(); assertEquals(Arrays.asList(1, 2, 3, 4, 5), result); + iterable = FluentIterable.of(1); + assertEquals(1, iterable.size()); + assertFalse(iterable.isEmpty()); + assertEquals(Arrays.asList(1), iterable.toList()); + result = FluentIterable.of(new Integer[0]).toList(); assertTrue(result.isEmpty()); final Iterable it = null; - result = FluentIterable.of(it).toList(); - assertTrue(result.isEmpty()); + try { + FluentIterable.of(it).toList(); + fail("expecting NullPointerException"); + } catch (NullPointerException npe) { + // expected + } } @Test @@ -151,9 +161,12 @@ public class FluentIterableTest { Collections.sort(combinedList); assertEquals(combinedList, result); - result = FluentIterable.of(iterableOdd).collate(null).toList(); - List expected = IterableUtils.toList(iterableOdd); - assertEquals(expected, result); + try { + FluentIterable.of(iterableOdd).collate(null).toList(); + fail("expecting NullPointerException"); + } catch (NullPointerException npe) { + // expected + } } @Test @@ -309,9 +322,12 @@ public class FluentIterableTest { Collections.sort(combinedList); assertEquals(combinedList, result); - result = FluentIterable.of(iterableOdd).zip((Iterable) null).toList(); - List expected = IterableUtils.toList(iterableOdd); - assertEquals(expected, result); + try { + FluentIterable.of(iterableOdd).zip((Iterable) null).toList(); + fail("expecting NullPointerException"); + } catch (NullPointerException npe) { + // expected + } result = FluentIterable.of(Arrays.asList(1, 4, 7)).zip(Arrays.asList(2, 5, 8), Arrays.asList(3, 6, 9)).toList(); combinedList = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9); @@ -354,7 +370,12 @@ public class FluentIterableTest { @Test public void size() { - assertEquals(0, FluentIterable.of((Iterable) null).size()); + try { + FluentIterable.of((Iterable) null).size(); + fail("expecting NullPointerException"); + } catch (NullPointerException npe) { + // expected + } assertEquals(0, FluentIterable.of(emptyIterable).size()); assertEquals(IterableUtils.toList(iterableOdd).size(), FluentIterable.of(iterableOdd).size()); }