views:

197

answers:

2

I have a strange problem where a for loop in PHP only returns the last item in my array.

The array is created with SimpleXML from an XML file.

The code should return this:

<tags><tag value="Tag1" /><tag value="Tag2" /><tag value="Tag3" /></tags>

But instead I just get:

<tags><tag value="Tag3" /></tags>

So it ignores all but the last item in the array no matter how many I got in there.

Can anyone see what I'm doing wrong?

Here's the code:

<?php

function gettags($xml)
{
    $xmltags = $xml->xpath('//var[@name="infocodes"]/string');
    return $xmltags[0];
}

//Path to the XML files on the server
$path = "/xmlfiles/";

//Create an array with all the XML files
$files = glob("$path/*.xml");

foreach($files as $file)
{
    $xml = simplexml_load_file($file);
    $xmltags = gettags($xml);

//Using the , character split the values coming from the $xmltags into an array
$rawtags = explode(',', $xmltags);

//Loop through the tags and add to a variable. Each tag will be inside an XML element - <tag value="tagname" />
for ($i = 0; $i <count($rawtags); $i++){
    $tags = '<tag value="' . $rawtags[$i] . '" />';
}

//Replace HTML escaped characters (ä, å, ö, Å, Ä, Ö) and the | character with normal characters in the tags variable
$tagsunwantedchars = array("&Ouml;", "&Auml;", "&Aring;", "&ouml;", "&auml;", "&aring;", "|");
$tagsreplacewith = array("Ö", "Ä", "Å", "ö", "ä", "å", " - ");
$tagsclean = str_replace($tagsunwantedchars, $tagsreplacewith, $tags);

//Create the full tag list and store in a variable
$taglist = "<tags>$tagsclean</tags>";

}

echo $taglist;

?>

Here's the XML file:

<wddxPacket version='1.0'>
    <header/>
    <data>
        <struct>
            <var name='infocodes'>
                <string>Tag1,Tag2,Tag3</string>
            </var>
        </struct>
    </data>
</wddxPacket>
+8  A: 

Simple bug: use $tags .= instead of $tags = in the loop:

$tags = '';
for ($i = 0; $i <count($rawtags); $i++){
    $tags .= '<tag value="' . $rawtags[$i] . '" />';
}
Matijs
The amount of times I've been caught out by this one!
RMcLeod
Wow such an easy fix! Thanks a lot.
Johannes
Alternatively, use `$tags[] = ...`, then `implode` after the loop. It should be a little more performant.
outis
It's also a good idea to initialize `$tags` before appending to it.
Ben James
Matijs
+2  A: 
for ($i = 0; $i <count($rawtags); $i++){
    $tags = '<tag value="' . $rawtags[$i] . '" />';
}

You're just overwriting the $tags variable on every iteration. Try:

$tags = '';
foreach ($rawtags as $rawtag) {
    $tags .= '<tag value="' . $rawtag . '" />';
}

Note that you should initialize $tags before appending to it (otherwise it will generate a PHP warning).

Also, using foreach instead of a for loop makes your code simpler and more readable. Trivial errors like this are easier to spot when they aren't surrounded by noise.

Ben James
Excellent - and how does he fix that?
Dominic Rodger
Thanks Ben. I'm a PHP novice and always appreciate help with making my code cleaner.
Johannes