tags:

views:

117

answers:

6

Hi, I've wrote a simple array, and I would use it to print an html list option set, with a selected element. My problem starts if I try to print multiple lists in my page, because only the first list is printed correctly, why?

<?php


$units = array (
'0' => 'Units',
'kJ' => 'Kilojoule: kJ',
'g' => 'Grams: g',
'mg' => 'Milligrams: mg',
'mcg' => 'Micrograms: mcg, µg');

function unit_select_option ($attributes, $code = "") {
    global $units;
    $html = "<select title=\"Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;\" $attributes>\r";

    while (list($key, $name) = each($units)) {
        if ($key == "0") {
            $html .= "  <option title=\"$name\" value='$key'>$name</option>\r";
        } else if ($key == $code) {
            $html .= "  <option title=\"$name\" selected=\"selected\" value='$key'>$key</option>\r"; 
        } else {
            $html .= "  <option title=\"$name\" value='$key'>$key</option>\r";
        }
    }
    $html.= "</select>\r";
    return $html;
}

print unit_select_option ('class="units_select"', "g");
print unit_select_option ('class="units_select"', "mg");
print unit_select_option ('class="units_select"', "mcg");
?>

the code shouldn't be nothing strange but I haven't found the issue because the page doesn't return any error.

html code:
<select title="Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;" class="units_select">
    <option title="Unit&agrave;" value='0'>Unit&agrave;</option>
    <option title="Kilojoule: kJ" value='kJ'>kJ</option>
    <option title="Grammi: g" selected="selected" value='g'>g</option>
    <option title="Milligrammi: mg" value='mg'>mg</option>
    <option title="Microgrammi: mcg, µg" value='mcg'>mcg</option>
</select>
<select title="Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;" class="units_select">
</select>
<select title="Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;" class="units_select">
</select>
+3  A: 

each() advances the internal array cursor. Because $units is a global variable, your first call of unit_select_option() advances the cursor to the end of $units, and there it remains for the subsequent calls.

You need to rewind your array using reset($units); at the end of unit_select_option().

PHP Documentation: reset

Pekka
the faster right answer should the winner! thats why I never win, or should be the clearest?
Vittorio Vittori
oh thanks all for the help!
Vittorio Vittori
+1  A: 

You need to reset your array after or before your while block.

Pikrass
+2  A: 

From each():

Return the current key and value pair from an array and advance the array cursor.

After each() has executed, the array cursor will be left on the next element of the array, or past the last element if it hits the end of the array. You have to use reset() if you want to traverse the array again using each.

So:

function unit_select_option ($attributes, $code = "") {
  global $units;
  $html = "<select title=\"Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;\" $attributes>\r";
  reset($units);
  while (list($key, $name) = each($units)) {
    if ($key == "0") {
      $html .= "  <option title=\"$name\" value='$key'>$name</option>\r";
    } else if ($key == $code) {
      $html .= "  <option title=\"$name\" selected=\"selected\" value='$key'>$key</option>\r"; 
    } else {
      $html .= "  <option title=\"$name\" value='$key'>$key</option>\r";
    }
  }
  $html.= "</select>\r";
  return $html;
}

I tend to avoid each() because its non-reentrant, meaning if inside your loop you call something else that uses it on the same array it'll affect your outer call. Not good. You tend to be better off just using a foreach loop:

function unit_select_option ($attributes, $code = "") {
  global $units;
  $html = "<select title=\"Kilojoule: kJ;&#13;Grammi: g;&#13;Milligrammi: mg;&#13;Microgrammi: mcg, µg;\" $attributes>\r";
  foreach ($units as $key => $name) {
    if ($key == "0") {
      $html .= "  <option title=\"$name\" value='$key'>$name</option>\r";
    } else if ($key == $code) {
      $html .= "  <option title=\"$name\" selected=\"selected\" value='$key'>$key</option>\r"; 
    } else {
      $html .= "  <option title=\"$name\" value='$key'>$key</option>\r";
    }
  }
  $html.= "</select>\r";
  return $html;
}

and you avoid all those issues.

cletus
ok, if you also give me an optimized version of my code, you must get the prize, thanks for help
Vittorio Vittori
+1  A: 

The other answers should have already solved your question.

I want to add that PHP has the foreach construct, so instead of the while loop you can just write

foreach ($unit as $key => $name) {
  ...
}

You don't need to reset() if you use foreach.

KennyTM
+3  A: 

You should reset the array pointer: use reset()

But why don't you use a foreach loop?

foreach($units as $key => $name){ ... }

And don't use global, it's evil. Declare $units array as static within the function body.

Crozin
+1 Curses - you beat me to it. :-)
middaparka
+1  A: 

Ok, as others have said the problem was because you weren't resetting the global array.

However, I'd be tempted not to use a global and instead pass it into the unit_select_option every time. (Arrays and objects are passed by reference in recent versions of PHP, so there's no reason not to do this and it's generally accepted as better programming practice.)

Secondly, you're doing some wierd things in the while loop - I'd have thought a foreach iterator would make more sense in this instance as such:

foreach($units as $key => $value)

Just an idea. :-)

middaparka