Pages system PHP/SQL

Question

I'm working on a CMS, currently on being able to create static pages, manage menus etc. so.. I have This function that reads from the DB and finds all the static pages, and their position in the menu. I want to know what Do you think.

function update_menu(&$pdo, $filepath){
$qry = "SELECT * FROM pages WHERE top = 1";
$exec = $pdo->query($qry);
$string = "<ul>";
while($row=$exec->fetch(PDO::FETCH_ASSOC)){
 $string.= "<li><a href='?page=".$row['id']."'>".$row['title']."</a>";
 $qry = "SELECT * FROM pages WHERE parent = ?";
 $child_ex = $pdo->prepare($qry);
 $child_ex->bindValue(1, $row['id'], PDO::PARAM_INT);
 if($child_ex->execute()){
 $i=0;
 while($child=$child_ex->fetch(PDO::FETCH_ASSOC)){
 if($i==0){
 $string.="<ul>";
 }
 $string.="<li><a href='?page=".$child['id']."'>".$child['title']."</a>";
 $i++;
 }
 if($i != 0)
 $string.="</ul>";
 }
 $string.="</li>";
}
$string.="</ul>";
file_put_contents($filepath, $string);

};

Output html:

<ul><li><a href='?page=1'>Page1</a><ul><li><a href='?page=4'>append1</a><li><a href='?page=5'>append2</a></ul></li><li><a href='?page=2'>page2</a><ul><li><a href='?page=6'>append1b</a></ul></li><li><a href='?page=3'>page3</a></li></ul>

SampleOutput

• Dave Jarvis: See: codereview.stackexchange.com/a/26810/20611
• Michael Zedeler: What is the purpose of this CMS? After all, there is a lot of PHP based CMS'es already, so why reinvent another?
• Carlos Arturo Alaniz: there's not propose, I like to do my own stuff plus its a good way of learning and gaining experience (Im a Cs student)
• SaggingRufus: @CarlosArturoAlaniz A quote a wise man once told me "Why are building your own framework?! Let me guess what you do is too special for one that already exists? No it not, why would you put yourself through what has already been done for you. What ever you build is not special"
• Félix Gagnon-Grenier: @SaggingRufus you guys miss some reflexion. Further pushing your point, why do you even speak since what you've just said has already been said so much times, and so much better then you did?

Answer 1

I think it overall is good, but there's some minor changes that could still quicken this script, namely using fetchAll and foreach instead of fetch and while

using fetchAll will get the values quicker then fetch on every single row. also foreach will prevent you from having to check if you have results, it just won't run if there are not.

also, you prepare the statement multiple times in the huge loop, instead of preparing it outside of the loop and execute() it multiple times

function update_menu(&$pdo, $filepath){
 $qry = "SELECT * FROM pages WHERE top = 1";
 $exec = $pdo->query($qry)->fetchAll(PDO::FETCH_ASSOC);
 $qryChild = $pdo->prepare("SELECT * FROM pages WHERE parent = ?");
 $string = '<ul>' . "\n";
 foreach($exec as $row)){
 $string.= "<li><a href='?page=".$row['id']."'>".$row['title'].'</a>' . "\n";
 $qryChild->execute($row['id']);
 $childs = $qryChild->fetchAll(PDO::FETCH_ASSOC);
 $i=0;
 foreach($childs as $child){
 if($i==0){
 $string.='<ul>' . "\n";
 }
 $string.="<li><a href='?page=".$child['id']."'>".$child['title'].'</a>' . "\n";
 $i++;
 }
 if($i != 0) $string.='</ul>' . "\n";
 $string.='</li>' . "\n";
 }
$string.='</ul>' . "\n";
file_put_contents($filepath, $string);

also, adding some "\n" will give some better readability to you html code