I've done a commit to CVS with the changes, plus some other recent tweaks. Thanks Bruno. Sean On Aug 17, 2006, at 9:55 PM, Bruno Van Den Bossche wrote: > Hello all, > > I believe to have found a bug in the implementation for the > iterator of > sim.util.Bag (current version in the CVS) > > The problem is located in the method remove() lines 454-461 > >> public void remove() >> { >> if (!canRemove) throw new IllegalStateException("remove() >> before next(), or remove() called twice"); >> // more consistent with the following line than 'obj > >> bag.numObjs' would be... >> if (obj - 1 >= bag.numObjs) throw new NoSuchElementException >> ("No More Elements"); >> bag.removeNondestructively(obj-1); >> canRemove = false; >> } > > The remove() method in an iterator is defined as such that it will > remove the element that was returned by the last next()-operation. > > As you can see in the code the field obj (an integer) points to the > next > element that can be accessed through the iterator. So to remove the > element the removeNondestructively method is called correctly with > 'obj - 1' as argument. > > However, obj itself should also decremented with one. If not, you > will > skip an element for each remove operation. > > For example, the following code will not function correctly: > > Bag bag = new Bag(); > bag.add(obj1); > bag.add(obj2) > > Iterator it = bag.iterator(); > while (it.hasNext()) { > Object o = bag.next(); > it.remove(); > } > > You will notice that the bag is not empty at after executing this > while, > although it should be. > > Fixing the problem can be done by decrementing the obj-field with > one in > the remove() method. > > > Kind Regards, > Bruno > > -- > Bruno Van Den Bossche > [log in to unmask] > http://www.ibcn.intec.UGent.be