Iterator.remove() - a gotcha

The JSE Iterator specification states that the implementation of remove() is optional and that if unimplemented an UnsupportedOperationException is thrown. I suspect that this can, and does, lead to problems as best practice tells us that you should always used a collection interface as opposed to its concrete implementation...

The MyCollectionFilter class is a simple utility class that’s uses some arbitrary criteria to filter a list, removing matching values from the input and placing them in the results list. Using Iterator.remove() looks a very elegant method of tackling this problem, but it will fail when the input List implementation is either CopyOnWriteArrayList or CopyOnWriteArraySet or some other Collection class where the iterator does not implement remove(). Note, that I’ve seen this algorithm used in production code...

  /**
   * A simple utility class used to filter two lists based upon some criteria.
   * @param <T>
   *            The filter type
   */
  public class MyCollectionFilter<T> {

    private final FilterCriteria<T> filter;

    /**
     * Create the filter - injecting the criteria
     *
     * @param filter
     *            The criteria
     */
    public MyCollectionFilter(FilterCriteria<T> filter) {
      this.filter = filter;
    }

    /**
     * Filter two lists, removing values from one and inserting into another based upon
     * some criteria.
     * @param items
     *            The items test.
     * @return The return list.
     */
    public List<T> filter(List<T> items) {

      List<T> results = new ArrayList<T>();
      Iterator<T> it = items.iterator();
      while (it.hasNext()) {
        T item = it.next();
        if (filter.test(item)) {
          results.add(item);
          it.remove();
        }
      }
      return results;
    }
  }

This problem is fixable, but you need to look at the root cause. The filter method breaks the encapsulation of the caller by changing the input argument. This can be avoided by creating a simple POJO as a return type that contains two new lists. This is better in that it’ll work with all collection classes. Its downside is that it’ll use more memory - which maybe another problem for another day...

    /**
     * Filter a list, dividing it based upon some arbitrary criteria, returning the two
     * separate lists as a single bean
     *
     * @param items
     *            The items test.
     * @return A bean containing the two output lists
     */
    public MyListBean filter(List<T> items) {

      List<T> filteredList = new ArrayList<T>();
      List<T> originalList = new ArrayList<T>();

      for (T item : items) {
        if (filter.test(item)) {
          filteredList.add(item);
        } else {
          originalList.add(item);
        }
      }

      return new MyListBean(filteredList, originalList);
    }

    /**
     * A return bean
     */
    public class MyListBean {

      private final List<T> filteredList;

      private final List<T> originalList;

      /**
       * Construct the bean
       *
       * @param filteredList
       * @param originalList
       */
      public MyListBean(List<T> filteredList, List<T> originalList) {
        this.filteredList = filteredList;
        this.originalList = originalList;
      }

      /**
       * @return the filteredList
       */
      public List<T> getFiltedList() {
        return filteredList;
      }

      /**
       * @return the originalList
       */
      public List<T> getOrginalList() {
        return originalList;
      }
    }