vote up 0 vote down star

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>
flag

75% accept rate

2 Answers

vote up 8 vote down check

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

$tags = '';
for ($i = 0; $i <count($rawtags); $i++){
    $tags .= '<tag value="' . $rawtags[$i] . '" />';
}
link|flag
The amount of times I've been caught out by this one! – RMcLeod Jan 6 at 9:37
Wow such an easy fix! Thanks a lot. – Johannes Jan 6 at 9:42
1  
Alternatively, use $tags[] = ..., then implode after the loop. It should be a little more performant. – outis Jan 6 at 9:44
It's also a good idea to initialize $tags before appending to it. – Ben James Jan 6 at 9:46
Thanks Dominique & Ben for the suggestions. – Matijs Jan 6 at 16:56
vote up 2 vote down
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.

link|flag
Excellent - and how does he fix that? – Dominic Rodger Jan 6 at 9:37
Thanks Ben. I'm a PHP novice and always appreciate help with making my code cleaner. – Johannes Jan 6 at 10:26

Your Answer

Get an OpenID
or

Not the answer you're looking for? Browse other questions tagged or ask your own question.