-1
\$\begingroup\$

I was given a problem where a Java program crashes 50 % of the time. It included this part

for (Object o : l) {
    for (Object o2 : l) {
        if (!o.equals(o2) && distance(o, o2) <= 1) {
            l.remove(o);
        }
    }

And the exception was a ConcurrentModificationException. So I rewrote it like this instead, then it doesn't crash, but is it correct?

Iterator<Object> itr = l.iterator();
while (itr.hasNext()){
    Object o = itr.next();
    for (Object o2 : l) {
        if (!o.equals(o2) && distance(o, o2) <= 1) {
            itr.remove();
            break;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ What is this l thing? What kind of objects are we dealing with? \$\endgroup\$ – Simon Forsberg Mar 19 '18 at 16:43
  • \$\begingroup\$ @SimonForsberg It is an ArrayList of geometric primitives e.g. Iterator<Figure> should be used instead. The code is from an assignment where they intentionally have gone too high up the class hierarchy. IIUC it should not be necessary to go so high as Object and the code could be more specific. \$\endgroup\$ – Niklas Mar 20 '18 at 0:53
1
\$\begingroup\$

Yes, it's safe, because you're following the rules for Iterator#remove:

Removes from the underlying collection the last element returned by this iterator (optional operation). This method can be called only once per call to next(). The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling this method.

Since you're breaking out on the first call to remove, then moving on, you'll never violate this rule.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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