Fix (small?) Error in Recursive Function

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
User avatar
Chalks
Forum Contributor
Posts: 447
Joined: Thu Jul 12, 2007 7:55 am
Location: Indiana

Fix (small?) Error in Recursive Function

Post by Chalks »

Here's the recursive function that I'm messing with:

Code: Select all

function display($names, $spacer="y")
{
print_r($names);  // Troubleshooting
  foreach($names AS $named)
    if(is_array($named))
    {
echo "is array<br />";  // Troubleshooting
      if($spacer=="y")
      {
echo "spacer is y<br />";  // Troubleshooting
        $dis .= substr($dis, 0, -6) . "\n<ul>\n";
        $spacer = "n";
      }
      $dis .= display($named, "y");
    }
    else
{
echo "added $named to dis<br />";  // Troubleshooting
      $dis .= "<li><a href=\"?$named\">" . $named . "</a></li>\n";
}
 
  if($spacer=="n")
    $dis .= "</ul></li>\n\n";
 
  return $dis;
}
I call the function 3 times with the following three arrays as input:

Code: Select all

Array ( [0] => e [1] => Array ( [0] => f [1] => Array ( [0] => g ) ) )
Array ( [0] => a [1] => Array ( [0] => b ) [2] => Array ( [0] => c ) )
Array ( [0] => d )

I have this as output:

Code: Select all

Array ( [0] => e [1] => Array ( [0] => f [1] => Array ( [0] => g ) ) ) added e to dis
is array
spacer is y
Array ( [0] => f [1] => Array ( [0] => g ) ) added f to dis
is array
spacer is y
Array ( [0] => g ) added g to dis
Array ( [0] => a [1] => Array ( [0] => b ) [2] => Array ( [0] => c ) ) added a to dis
is array
spacer is y
Array ( [0] => b ) added b to dis
is array
Array ( [0] => c ) added c to dis
Array ( [0] => d ) added d to dis
total output:
 
# e
# e
 
    * f
    * f
          o g
 
# a
# a
 
    * b
    * c
 
# d
It should look like this:

Code: Select all

# e
 
    * f
          o g
 
# a
    * b
    * c
 
# d
The error only seems to occur if the parent element has any children. Why are there two entries for e, two for f, and two for a? Each is only added once to the output. Where are those extra ones coming from?
User avatar
Chalks
Forum Contributor
Posts: 447
Joined: Thu Jul 12, 2007 7:55 am
Location: Indiana

Re: Fix (small?) Error in Recursive Function

Post by Chalks »

Added a reply because the original post was long enough.

this tiny bit of code "substr($dis, 0, -6)" is what's messing it all up. Take that out and it works fine. Why does that little bit of code screw it up so badly?
User avatar
ghurtado
Forum Contributor
Posts: 334
Joined: Wed Jul 23, 2008 12:19 pm

Re: Fix (small?) Error in Recursive Function

Post by ghurtado »

Change

Code: Select all

$dis .= substr($dis, 0, -6) . "\n<ul>\n";
to

Code: Select all

$dis = substr($dis, 0, -6) . "\n<ul>\n";
Declaring your variables at the top of your function could have helped you see this issue.
User avatar
ghurtado
Forum Contributor
Posts: 334
Joined: Wed Jul 23, 2008 12:19 pm

Re: Fix (small?) Error in Recursive Function

Post by ghurtado »

Crossposted :)

I dont think it is so much the substring function, but the fact that you keep concatenating a variable to itself, twice. Once inside the function and once when the function calls itself.
User avatar
Chalks
Forum Contributor
Posts: 447
Joined: Thu Jul 12, 2007 7:55 am
Location: Indiana

Re: Fix (small?) Error in Recursive Function

Post by Chalks »

It _is_ the substring function. Leaving everything exactly the same and just removing the substring works perfectly (though the effect that I was trying to get with substring is gone now). I believe the issue was that it was trying to do substr() on a string that had 0 length (when substr was called, $dis would not have been initialized). Here's the final function that performs everything I needed it to, eliminating the need for substr().

Code: Select all

function display($names, $spacer="y")
{
  foreach($names AS $named)
    if(is_array($named))
    {
      if($spacer=="y")
      {
        $dis .= "<!--[if IE 7]><!--></a><!--<![endif]--><!--[if lte IE 6]><table><tr><td><![endif]--><ul>\n";
        $spacer = "n";
      }
      $dis .= display($named, "y");
    }
    else
      $dis .= "\n<li><a href=\"?$named\">" . $named . "</a></li>";
 
  if($spacer=="n")
    $dis .= "</ul><!--[if lte IE 6]></td></tr></table></a><![endif]--></li>\n\n";
 
  return $dis;
}
I could take out the .= on lines 8 and 14, and it would work fine. Leaving them in makes no difference though (I've tested both ways). Then after everything is returned, I just use string replace to get rid of the extra </a></li> (that's what substring was supposed to do originally) and everything works fine.

As for declaring the variable at the top of the function, I didn't want to initialize them to anything because that would defeat the purpose of a recursive function. Normally I do have them up there, but not in this case.
User avatar
ghurtado
Forum Contributor
Posts: 334
Joined: Wed Jul 23, 2008 12:19 pm

Re: Fix (small?) Error in Recursive Function

Post by ghurtado »

Chalks wrote: As for declaring the variable at the top of the function, I didn't want to initialize them to anything because that would defeat the purpose of a recursive function.
I think you may be a little bit confused about how a recursive function works. Every time the function calls itself, you are creating a new $dis variable, which only becomes the value of the old $dis variable (the one in the calling function) because you return it. In my opinion, declaring the variable at the top is a good idea, since in your case it could have helped you "visualize" the fact that with every recursive call you had a new $dis variable, and not the one you were using before.
Post Reply