33

CollectionUtils::removeAll() Commons Collections 3.2.1

I must be going crazy, becuase it seems like this method is doing the inverse of what the docs state:

Removes the elements in remove from collection. That is, this method returns a collection containing all the elements in c that are not in remove.

This little JUnit test

@Test
public void testCommonsRemoveAll() throws Exception {
    String str1 = "foo";
    String str2 = "bar";
    String str3 = "qux";

    List<String> collection = Arrays.asList(str1, str2, str3);
    System.out.println("collection: " + collection);

    List<String> remove = Arrays.asList(str1);
    System.out.println("remove: " + remove);

    Collection result = CollectionUtils.removeAll(collection, remove);
    System.out.println("result: " + result);
    assertEquals(2, result.size());
}

Is failing with

java.lang.AssertionError: expected:<2> but was:<1>

and prints

collection: [foo, bar, qux] 
remove: [foo] 
result: [foo]

From my reading of the docs I should expect [bar, qux]. What have I missed?

skaffman
  • 398,947
  • 96
  • 818
  • 769
markdsievers
  • 7,151
  • 11
  • 51
  • 83
  • I updated my post to reflect this since someone reminded me of it - but Apache Commons Collections 4.0 was released in November 2013, with a fix for this issue. – wkl Jan 02 '14 at 14:31

1 Answers1

45

Edit January 1, 2014 Apache Commons Collections 4.0 was finally released on November 21, 2013, and contains a fix for this issue.

Link to CollectionUtils.java

Lines in question (1688 - 1691), with acknowledgement the method was previously broken:

/*
 ...
 * @since 4.0 (method existed in 3.2 but was completely broken)
 */
public static <E> Collection<E> removeAll(final Collection<E> collection, final Collection<?> remove) {
    return ListUtils.removeAll(collection, remove);
}

Original Answer

Nope, you're not crazy. removeAll() is actually (incorrectly) calling retainAll().

This is a bug in CollectionUtils, affecting version 3.2. It's been fixed, but only in the 4.0 branch.

https://issues.apache.org/jira/browse/COLLECTIONS-349

And as further proof, here's a link to the source code:

http://svn.apache.org/repos/asf/commons/proper/collections/tags/COLLECTIONS_3_2/src/java/org/apache/commons/collections/CollectionUtils.java

Check out this line:

public static Collection removeAll(Collection collection, Collection remove) {
    return ListUtils.retainAll(collection, remove);
}

Yep...broken!

wkl
  • 77,184
  • 16
  • 165
  • 176
  • 1
    Holy smokes! How did that slip through the cracks? Thanks for the info. Upvote and accept for you. – markdsievers Dec 04 '11 at 01:49
  • @markdsievers - Looks like either unit tests are needed, or need fixing! – wkl Dec 04 '11 at 01:50
  • IMO, this is pretty poor. Mistakes are OK, but the original issue has a creation stamp of "02/Aug/06 17:37", and they STILL haven't made a production release with the fix in it. – Stephen C Dec 04 '11 at 02:28
  • @StephenC - It's extremely poor, even a patch release would be better than waiting for a release of the trunk, and the last time Apache Commons Collections saw a release was version 3.2.1 in April 2008. Maybe no one thinks this is a big deal, or they don't know it's broken and there are problems in code bases everywhere. – wkl Dec 04 '11 at 03:04
  • 2
    For anyone interested, my solution was to use the intent of this method directly, ie ListUtils.removeAll(a,b). – markdsievers Dec 04 '11 at 06:25