Can't use remove() in a loop

Bug #458371 reported by Anthony Bush on 2009-10-22
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
CoughPHP
Medium
Anthony Bush
1.4
Medium
Anthony Bush

Bug Description

The following code does not work (for collections containing more than one element):

 <?php
 foreach ($collection as $element) {
  $collection->remove($element);
 }
 assert(count($collection) == 0);

Only the first element is removed. However, this code does work:

 <?php
 foreach ($collection as $element) {
  $collection->offsetUnset($element);
 }
 assert(count($collection) == 0);

and this works too:

 <?php
 foreach ($collection as $element) {
  unset($collection[$element]);
 }
 assert(count($collection) == 0);

I've found that the reason for this is due to CoughCollection saving the removed element (for later processing) AFTER the unset is performed.

 $this->offsetUnset($key);
 $this->removedElements[] = $objectToRemove;

Changing this to:

 $this->removedElements[] = $objectToRemove;
 $this->offsetUnset($key);

fixes the problem. There's two places in CoughCollection to fix, removeByKey(), and removeByReference().

Anthony Bush (awbush) on 2009-11-05
Changed in coughphp:
status: Confirmed → Fix Committed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers