How (not) to remove items from a (CDK) list

So there I was, trying to remove all mappings from a Reaction like this:
for (int i = 0; i < reaction.getMappingCount(); i++) { reaction.removeMapping(i); }
and found that only half the mappings were being removed ... can you see why? :)

In fact, this is not some obscure CDK bug, but a logic error on my part. Equivalent code is this:
List list = getListSomehow();
for (int i = 0; i < list.size(); i++) { list.remove(i) }
using for example a java.util.ArrayList. The problem is that the index (i) is being tested against a changing number (the size). Once half the items have been removed, i is at the half-way point, so on the next pass it stops.

One way to 'solve' this is to go backwards:
for (int i = list.size(); i > 0; i--) { reaction.removeMapping(i); }
but this is slightly less clear than just :
List list = getListSomehow();
int size = list.size();
for (int i = 0; i < size; i++) { list.remove(i) }
which is clearer. Of course, even better is the List method removeAll(). It would be nice if Reaction had a similar method...


IAtomContainer has removeXX() methods... I see no reason why IReactionXX cannot have the same.
Rajarshi said…
Which underscores the idea that list like objects should extend List :)
gilleain said…
Also, setXX - neither reaction nor mapping objects have this.

@Rajarshi : of course, a nice language (like - say - python) can extend types. Being able to say "public class AtomContainer(List) {" would be great :)