17

I have an ArrayList to be filtered, and various Guava Predicates to filter it with. This list will have only 50-100 elements.

I was planning on Iterables.removeIf using each predicate in turn. It is perhaps not maximally efficient but never mind (at least removeIf has some optimization for RandomAccess lists)

For debugging, I want to concisely log what each predicate did. e.g.

Pred0 removed [a, c, g]
Pred1 removed []
Pred2 removed [b, f]

There are some obvious hack solutions but what would you suggest as the cleanest?

For bonus points, it should be reasonably efficient too. ;)

Iain
  • 1,797
  • 1
  • 20
  • 38

5 Answers5

33

I would capture the removed elements in your Predicate code.

List<String> removedElements = Lists.newArrayList();
final Iterables.removeIf(list, new Predicate<String>() {
    @Override
    public boolean apply(String input) {
        if ("a".equals(input)) {
            removedElements.add(input);
            return true;
        }
        return false;
    }
}); 
JanRavn
  • 1,002
  • 8
  • 12
  • 3
    Thanks, this is what I ended up with too - I like its simplicity. I am sure it is not 100% kosher for a Predicate to have side effects, but I don't really care (we could make it idempotent etc. if we did care) – Iain Jun 29 '11 at 08:17
5

This may be an occasion when using a loop is simplest.

List<MyType> list =
Predicate<MyType>[] predicates =
Map<Predicate, List<MyType>> removed = 
      new LinkedHashMap<Predicate, List<MyType>>();
for(Iterator<MyType> iter=list.iterator();list.hasNext();) {
   MyType mt = iter.next();
   for(Predicate<MyType> pred: predicates) 
       if(pred.apply(mt)) {
          List<MyType> mts = removed.get(pred);
          if(mts == null)
              removed.put(pred, mts = new ArrayList<MyType>());
          mts.add(mt);
          iter.remove();
          break;
       }
 }
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 4
    A `Multimap` would make things more concise there too. – ColinD Jun 27 '11 at 15:05
  • Thanks, I can see this works. ...It kinda makes me shudder to be `remove`'ing things from an `ArrayList.iterator()` but I suppose no worse in practice, for these small list sizes. (I was interested to see there is no custom strategy for `ArrayList.iterator().remove()` the way that Guava does in `Iterables.removeIf`). – Iain Jun 29 '11 at 08:14
4

I'd investigate in observable predicates. The idea: everytime, a predicates apply method is about to return true, it will fire a notification to listeners:

Iterable<?> iterable = getIterable();
Collection<ObservablePredicate> predicates = getPredicates();
PredicatesLogger log = new PredicatesLogger(predicates);  // listens to all predicates
for (ObservablePredicate pred : predicates) {
  Iterables.removeIf(iterable, pred);
  log.print();
  log.reset();
}

The ObservableLogger is a decorator for Predicate :

public class ObservableLogger implements Predicate {
  private Predicate predicate;

  private List<Listener> listeners = new ArrayList<Listener>();
  // usual stuff for observer pattern

  @Override
  public boolean apply(Object input) {
    boolean result = predicate.apply(input);
    fire(result);
    return result;
  }

  // a fire method
}

The PredicateLogger needs one constructor that adds itself as a listener to the predicates. It will receive the notifications and cache the predicates that fired the events (the Event class needs an appropriate field for that information). print will create the log message, reset will clear the loggers cache (for the next run).

Andreas Dolk
  • 113,398
  • 19
  • 180
  • 268
  • Thanks for the idea. For my single case it is probably too gold-plated. No point building reusable infrastructure when I may never need it again... – Iain Jun 29 '11 at 08:26
  • 2
    @Iain - what about some gold-plate for your skills :-D – Andreas Dolk Jun 29 '11 at 08:31
1

I agree with Peter's anwser.

But if you want to have fun, you could also wrap each of your predicate inside a predicate which would delegate to the wrapped predicate, and store the values for which true is returned inside a Map<Predicate, List<Foo>> removedByPredicate :

class PredicateWrapper implements Predicate<Foo> {
    private Predicate<Foo> delegate;

    public PredicateWrapper(Predicate<Foo> delegate) {
        this.delegate = delegate;
    }

    @Override
    public boolean apply(Foo foo) {
        boolean result = delegate.apply(foo);
        if (result) {
            List<Foo> objectsRemoved = removedByPredicate.get(delegate);
            if (objectsRemoved == null) {
                objectsRemoved = Lists.newArrayList();
                removedByPredicate.put(delegate, objectsRemoved);
            }
            objectsRemoved.add(foo);
        }
        return result;
    }
}
JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Ah yeah same idea as @JanRavn - thanks. (I like @PeterLawrey's concise two-liner way of managing a multimap; never thought of it...) – Iain Jun 29 '11 at 08:23
-4

I guess you need:

Predicate<XXX> predicate1 = new Predicate<XXX>(){  
    @Override  
    public boolean apply(XXX input) {  
        if(...) //satisfy your filter
            return true;  
        else  
            return false;  
}};  

Predicate<XXX> predicate2 = new Predicate<XXX>(){  
    @Override  
    public boolean apply(XXX input) {  
        if(...) //satisfy your filter
            return true;  
        else  
            return false;  
}};
Predicate allPredicates = Predicates.and(predicate1, predicate2);
//or Predicates.or(predicate1, predicate2);

Collection<XXX> list2 = Collections2.filter(list, allPredicates); 
卢声远 Shengyuan Lu
  • 31,208
  • 22
  • 85
  • 130