1

I've got a foreach loop that is only running once and it has me stumped.

1: I load an array of status values (either "request", "delete", or "purchased")

2: I then load an XML file and need to loop through the "code" nodes and update their status, but if the new code is "delete" I want to remove it before moving onto the next one

The XML structure is...

<content>
    .... lots of stuff
    <codes>
    <code date="xxx" status="request">xxxxx</code>
        .. repeat ...
    </codes>
</content>

And the PHP code is...

$newstatus = $_POST['updates'];
$file = '../apps/templates/' . $folder . '/layout.xml';
$xml2 = simplexml_load_file($file);
foreach($xml2->codes->code as $code) {
    if($code['status'] == "delete") {
        $dom = dom_import_simplexml($code);
        $dom->parentNode->removeChild($dom);
    }
}
$xml2->asXml($file);

I've temporarily removed the updating, so I can debug the delete check.

This all works, but it only removes the first delete and leaves all the other deletes even though it's a foreach loop?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
user2086003
  • 67
  • 1
  • 2
  • 5
  • Duplicate question, check the answer at http://stackoverflow.com/questions/3418197/how-to-remove-a-node-if-it-exists-with-simplexml?rq=1. – Rolando Isidoro Jun 20 '13 at 20:41
  • It is likely you have been copied over this code from [the (right now) accepted answer](http://stackoverflow.com/a/262556/367456) of *[Remove a child with a specific attribute, in SimpleXML for PHP](http://stackoverflow.com/q/262351/367456)*. Problem with that is, the answer given is unstable. The problem with that code is, that it is written for a single delete only as after the delete the iterator has changed. You need to convert it to an array first either by `iterator_to_array` or in case of simplexml in specific by using xpath. http://stackoverflow.com/a/16062633/367456 – hakre Jun 22 '13 at 08:09

2 Answers2

13

Deleting multiple times in the same iteration is unstable. E.g. if you remove the second element, the third becomes the second and so on.

You can prevent that by storing the elements to delete into an array first:

$elementsToRemove = array();
foreach ($xml2->codes->code as $code) {
    if ($code['status'] == "delete") {
        $elementsToRemove[] = $code;
    }
}

And then you remove the element based on the array which is stable while you iterate over it:

foreach ($elementsToRemove as $code) {
    unset($code[0]);
}

You could also put the if-condition into an XPath query which does return the array directly (see the duplicate question for an example) or by making use of iterator_to_array().

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
hakre
  • 193,403
  • 52
  • 435
  • 836
  • 1
    This is a really helpful answer. I've twice been in this situation and both times this has saved me. Maybe next time I might actually remember it... – PaulSkinner Oct 28 '14 at 13:54
  • Thank you for this, it worked with code like this. **$xml = new SimpleXMLElement($rs_xml); $elementsToRemove = array(); foreach($xml->channel->item as $element){ if ($element->guid == "http://www.EXAMPLE.com/?p=2385281") { $elementsToRemove[] = $element; } } foreach ($elementsToRemove as $code) { unset($code[0]); }** – ziggrat Jun 25 '15 at 09:34
0

SimpleXML node lists are plain arrays of references, and like with any deleting of items while forward iterating through an array, the array position pointer can get mixed up because the expected next item has disappeared.

The simple way to remove a bunch of children in SimpleXML without using an extra array is to iterate in reverse (=decrementing the index), taking the looping in your example to:

// For each node in reverse
$elements = $xml2->xpath('codes/code');
$count = count($elements);
for($j=$count-1; $j>=0; $j--) {
  // If to delete
  $code = $elements[$j];
  if($code['status'] == "delete") {
    // Delete element
    $dom = dom_import_simplexml($code);
    $dom->parentNode->removeChild($dom);
  }
}

Of course, if your other processing requires forward iterating through the elements, then using an array is the best.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Patanjali
  • 893
  • 13
  • 17