Print

Print


Hi Bruno. I am in Japan right now (attending WCSS) and will check on the
remove() error.   From my cursory glance at your code example, it looks
like you are correct.  This means there is probably an error in IntBag and
DoubleBag as well; I'll get fixed versions up on CVS when I get back to
the US, and probably have a new version of MASON out as well.

Sean, who didn't catch this as he never uses remove()... :-)


On Thu, 17 Aug 2006, 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
>
>