4
\$\begingroup\$

I was attempting to remove items from an ArrayList while iterating through it, only to find out that Java doesn't like that. So I quickly wrote up a Delay that I could pass operations to so I could run them later.

import java.util.HashSet;
import java.util.Set;

public class Delay {
    private Set<Runnable> tasks = new HashSet<Runnable>();

    public Delay(){}
    public Delay(Runnable... tasks){
        for(Runnable task : tasks) this.tasks.add(task);
    }

    public void add(Runnable task){
        this.tasks.add(task);
    }
    public void run(){
        for(Runnable task : tasks) task.run();
        tasks.clear();
    }
}

Code in use (in a card game):

Delay delay = new Delay();

...

// if a hand has no cards, remove it from play
for(Hand hand : hands) if(hand.size() == 0) delay.add(() -> hands.remove(hand));
delay.run();

Is there a flaw in my solution? Or does Java already have a method of handling this, making my code redundant? Any points of improvement?

\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

Since you are using java 8, there's the removeIf method (javadoc).

You can use it like this:

hands.removeIf(h -> h.size() == 0);

Or, if Hand is a Collection of some sort,

hands.removeIf(Hand::isEmpty)
\$\endgroup\$
7
  • \$\begingroup\$ That's wonderful, especially since Iterators are giving me a hard time! I learned on Java 7 and have been picking up pieces of 8 (no pun intended) for a while now. Also not terribly immersed in higher level techniques; lambda functions still kinda blow my mind! \$\endgroup\$
    – ndm13
    Sep 29, 2015 at 1:06
  • \$\begingroup\$ Also yes, Hand is a List actually, so that works out well! \$\endgroup\$
    – ndm13
    Sep 29, 2015 at 1:07
  • 1
    \$\begingroup\$ removeIf(Predicate) should be the right approach, but once again it is implementation-dependent when it comes to concurrent modification... As the Javadoc goes, the default implementation falls back to the Iterator approach, and ArrayList may still throw ConcurrentModificationException. \$\endgroup\$
    – h.j.k.
    Sep 29, 2015 at 1:57
  • 1
    \$\begingroup\$ I don't think OP's question is about the list being modified from another thread. He is just trying to remove elements from the list based on the predicate, which is exactly what removeIf does. @ndm, if I misunderstood your question and you ARE modifying the list from another thread, please say so. It would certainly change the answer. \$\endgroup\$
    – Misha
    Sep 29, 2015 at 2:13
  • 2
    \$\begingroup\$ @ndm Adding or removing elements to a collection is never thread-safe without some specific guarantees either from the specific collection implementation or from external synchronization you do yourself. \$\endgroup\$
    – Misha
    Sep 29, 2015 at 2:23
4
\$\begingroup\$

Yes java already has that. What you're looking for is an Iterator:

Iterator<Hand> handIt = hands.iterator();
while (it.hasNext()) {
    Hand current = handIt.next();
    if (current.size() == 0) {
        handIt.remove();
    }
}

This can be simplified / changed to:

for (Iterator<Hand> handIt = hands.iterator(); handIt.hasNext(); ) {
    Hand current = handIt.next();
    // ...
}
\$\endgroup\$
2
  • \$\begingroup\$ Hey, thanks a lot! I've used Iterators before, but for some reason thought they fell prey to the same cocurrent modification issues that Lists had. Thanks for the tip! \$\endgroup\$
    – ndm13
    Sep 28, 2015 at 20:52
  • 1
    \$\begingroup\$ @ndm13 an Iterator is backed by the List implementation, so using a concurrent-friendly one like CopyOnWriteArrayList may be something of interest. \$\endgroup\$
    – h.j.k.
    Sep 29, 2015 at 1:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.