MySQL DB driven menu generator function

Question

Recently I've written recursive PHP function which generates website navigation based on parent-child structure like this

<ul>
  <li>parent
     <li>child</li>
  </li>
</ul>

Code looks like that

function generateMenu($parent, $level, $db){
  $q = $db->query("select id, name FROM menu WHERE parent = '$parent'");

  if($level > 0 && $q->num_rows > 0) echo "\n<ul>\n";

   while($row=$q->fetch_object()){
      echo "<li>";
      echo '<a href="?page=' .$page. '">' . $row->name . '</a>';
      //display this level's children
      generateMenu($row->id, $level++, $menu, $db);
      echo "</li>\n\n";
    }

    if($level > 0 &&  $q->num_rows > 0) echo "</ul>\n";
}

The piece of code above simply echoes <ul><li> structure (like given above example) from db table.

Now the questions is, how to create navigation menu like on this website?

Please take a look at left sidebar.

http://www.smithsdetection.com/continuous_vapour_sampling.php

Now i think that:

First of all we need to echo all parents
Function must get current pages id as an input value (for ex. $current)
Function must echo til' current pages level

I can't figure out how to modify my function, to get output like on given website. PLease help.

BTW

My db table looks like that

enter image description here

NOTE Please don't post answers about sql injection holes, I've already taken care about them: checking with in_array (if variable listed in column names array) and passing through real_escape.

generateMenu($row->id, $level++, $menu, $db); should probably be generateMenu($row->id, $level+1, $menu, $db);. — daiscog
– daiscog, Commented Oct 7, 2011 at 12:27
I am aware of the increment operator, but you are using $level after it is incremented, too. Consider when $level = 0: if($level > 0 && $q->num_rows > 0) echo "\n<ul>\n"; will NOT output an opening <ul>, then later generateMenu($row->id, $level++, $menu, $db); increments $level to 1 so that after your while loop, if($level > 0 && $q->num_rows > 0) echo "</ul>\n"; WILL output a closing </ul>, which is not matched to any opening one. Check your HTML that gets returned to see. — daiscog
– daiscog, Commented Oct 7, 2011 at 12:58
Furthermore... if $level = 0, when you call generateMenu($row->id, $level++, $menu, $db) $level actually gets incremented after you pass it to the function, so the function always sees $level = 0. Compare: $level++ with ++$level, and read the manual link you posted above. — daiscog
– daiscog, Commented Oct 7, 2011 at 13:05

Philipp Spiess · Accepted Answer · 2011-10-07 12:22:52Z

3

Have a look at this: http://www.ferdychristant.com/blog/archive/DOMM-7QJPM7

You should try to fetch the whole hierarchy with one query to fix performance issue.

answered Oct 7, 2011 at 12:22

Philipp Spiess

3,2675 gold badges28 silver badges35 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

heron Over a year ago

is my question about performance problems? it's not the answer. please comment it or delete

Philipp Spiess Over a year ago

than just have a look at the blog post. your question is about hierarchical data, and there is a solution for it.

Mike B Over a year ago

@user978733 You're a bundle of joy

daiscog Over a year ago

Or (assuming the menu table does not get changed too often) index the parent column. That way the difference in approaches will be very small.

daiscog · Accepted Answer · 2011-10-07 13:55:41Z

2

Assuming the current page id is in the var $current and that $db is an open MySQLi DB connection:

// first get your current page's path back to root:
// $stack will be a stack of menus to show
$stack = array();

// always expand current menu:
$stack[] = $current;

// now starting at $current, we go through the `menu` table adding each parent
// menu id to the $stack until we get to 0:
$i = $current;
while ( $i > 0 ) {
  // get parent of $i
  $query = sprintf('SELECT `parent` FROM `menu` WHERE id=%d LIMIT 1', $i);
  $result = $db->query($query);

  if (!$result) {
    // do error handling here
  }

  $row = $result->fetch_assoc();

  // save parent id into $i...
  $i = $row['parent'];

  // ...and push it onto $stack:
  $stack[] = $i;
}

/**
 * @param int $parent the parent ID of the menu to draw.
 * @param array $stack the stack of ids that need to be expanded
 * @param string $indent string for pretty-printing html
 * @param MySQLi $db Open db connection
 */
function generateMenu($parent, $stack, $indent, $db){

  // $next is the next menu id that needs expanding
  $next = array_pop($stack);

  $query = sprintf('SELECT `id`, `name` FROM `menu` WHERE `parent`=%d', $parent);

  $result = $db->query($query);

  if ( ! $result ) {
    // do error handling here
  }

  if ($result->num_rows > 0) {
    echo "\n$indent<ul>\n";

    while($row = $result->fetch_object()){
      echo "$indent  <li>\n";
      echo "$indent    <a href=\"?page={$row->id}\">{$row->name}</a>\n";

      //display this level's children, if it's the $next menu to need to be drawn:
      if ($row->id == $next)
        generateMenu($next, $stack, "$indent    ", $db);

      echo "$indent  </li>\n\n";
    }
    echo "$indent</ul>\n";
  }
  $result->free();
}

$first = array_pop($stack); // should always be 0
generateMenu($first, $stack, '', $db);

edited Oct 7, 2011 at 13:55

answered Oct 7, 2011 at 12:55

daiscog

12.2k8 gold badges55 silver badges63 bronze badges

5 Comments

heron Over a year ago

I don't understand this part $stack[] = $current; $i = $current; $current is the current pages id as we declared above right?

daiscog Over a year ago

Yes. We need to add $current to the stack of menus to display. $stack[] = $current; does that. We are then using $i to travel backwards through the DB finding all ancestors of $current. The first ancestor we are looking for is the parent of $current, so we set $i = $current then in the while loop we execute a query to find the parent id, then change $i to = the parent id, then push the parent id onto the stack and if the parent id (i.e., $i) is > 0, we do it again. Looking at your table, if $current = 12, then after the while loop, $stack = array(12, 10, 4, 0);

daiscog Over a year ago

Try following it through, line-by-line, and writing down what each variable is at each line and I'm sure you'll see what happens.

heron Over a year ago

how to set as active current oage? class=active i mean

daiscog Over a year ago

Just add if ($row->id == $current) in the while loop. Do you want to add the class to the <li> or or <a>?

Collectives™ on Stack Overflow

MySQL DB driven menu generator function

2 Answers 2

4 Comments

5 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related