[java] [6.0.0] ForLoopCanBeForeach false-positive

[boris-petrov]

for (Iterator<E> iterator = iterator(), otherIterator = other.iterator(); iterator.hasNext();) {
	E item = iterator.next();
	E otherItem = otherIterator.next();
	...
}

This code should not give a ForLoopCanBeForeach warning. At least I can't see another way of going through a couple of collections at the same time with a foreach. Am I wrong?

[oowekyala]

Thanks for the report! I wrote the rule and just didn't think about that use case for the for loop. From the looks of it, we could say that loops with two iterator declarations are not replaceable. This looks easy enough to fix

[oowekyala]

Technically, I think the rule isn't wrong, you could refactor it like that:

Iterator<E> otherIterator = other.iterator();
for (E item : this) {
  E otherItem = otherIterator.next();
}

Which is moderately easier to read, but puts otherIterator in the outer scope... Should we recommend this?

[boris-petrov]

@oowekyala - as you pointed out, otherIterator is in the outer scope which is not nice. Also, this implicitly requires otherIterator to have more (or same number of) elements than this - in my example above I have a check before the loop that they have equal sizes, but maybe one would want to write:

for (Iterator<E> iterator = iterator(), otherIterator = other.iterator(); iterator.hasNext() && otherIterator.hasNext();) {

Which would only cycle through elements until one of the lists is exhausted. I still think this is a false-positive.

[jsotuyod]

I agree with @boris-petrov. To manually use and advance both iterators in the same way at the same time makes it more explicit they are bound together.

For most scenarios having two separate but related iterators may not be the best structure, but for some algorithms this makes perfect sense (ie: intersect / diff / union of different sorted collections).