Must be an easier/more efficient way to code this

beckerdt · April 2, 2011, 5:05am

Hello, I’m new to PHP and I have come up with the following code for my navigation but I am sure there has to be an easier and more efficient way to do this. Someone please start me on the right path. Thanks

<ul>
<?php
	//run the loop to get the page titles  
	$query = "SELECT * FROM pages ORDER BY position ASC";
		$page_set = mysql_query($query);
			confirm_query($page_set);
				while($page = mysql_fetch_array($page_set)){
					echo '<li><a href="content.php?page=' . urlencode($page['id']) . '">' . $page['page_name'] . '</a></li>';
						//run sub page query to see if the ul tag is needed
						$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
							$sub_page_set = mysql_query($query);
								confirm_query($sub_page_set); 
									$sub_page = mysql_fetch_array($sub_page_set);
										if ($sub_page !=''){
												echo '<ul>';
												}
									//run the sub page loop again to display the sub page title
									$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
										$sub_page_set = mysql_query($query);
											confirm_query($sub_page_set); 
									while($sub_page = mysql_fetch_array($sub_page_set)) {
												echo '<li><a href="content.php?sub_page=' . urlencode($sub_page['id']) . '">' . $sub_page['page_name'] . '</a></li>';
									}
											//run the sub page loop for the last time to see if the end ul tag is needed
											$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
												$sub_page_set = mysql_query($query);
													confirm_query($sub_page_set); 
											$sub_page = mysql_fetch_array($sub_page_set);
												if ($sub_page !='') {
														echo '</ul>';
													}
					}
?>	
</ul>

Cups · April 2, 2011, 1:59pm

It looks as though you have 2 tables.

pages
sub_pages

What is in the function confirm_query() ?

beckerdt · April 2, 2011, 3:25pm

Function:
function confirm_query($result){
if (!$result) {
die('Query failed: ’ . mysql_error());
}
}

tangoforce · April 2, 2011, 3:33pm

For a start you could reduce your indentation so you can actually read it without scrolling all over the place. I’d recommend 3 spaces per indent some use as little as 2 and I’d say the max is 4.

beckerdt · April 2, 2011, 3:54pm

Sorry I hope this is better.


<?php 
//Get selected page/sub page
	if (isset($_GET['page'])) {
	$sel_page = $_GET['page'];
	$sel_sub_page = '';
	} elseif (isset($_GET['sub_page'])) {
	$sel_page = '';
	$sel_sub_page = $_GET['sub_page'];
	} else {
	$sel_page = '';
	$sel_sub_page = '';
		}
?>
<body>
	<!--Begin Navigation-->
<ul>
<?php
	//run the loop to get the page titles  
	$query = "SELECT * FROM pages ORDER BY position ASC";
	$page_set = mysql_query($query);
	confirm_query($page_set);
	   while($page = mysql_fetch_array($page_set)){
	      echo '<li><a href="content.php?page=' . urlencode($page['id']) . '">' . $page['page_name'] . '</a></li>';
	//run sub page query to see if the ul tag is needed
	$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
	$sub_page_set = mysql_query($query);
	confirm_query($sub_page_set); 
	$sub_page = mysql_fetch_array($sub_page_set);
	 if ($sub_page !=''){
		echo '<ul>';
		}
	//run the sub page loop again to display the sub page title
	$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
	$sub_page_set = mysql_query($query);
	confirm_query($sub_page_set); 
	while($sub_page = mysql_fetch_array($sub_page_set)) {
	echo '<li><a href="content.php?sub_page=' . urlencode($sub_page['id']) . '">' .   $sub_page['page_name'] . '</a></li>';
		}
	//run the sub page loop for the last time to see if the end ul tag is needed
	$query = "SELECT * FROM sub_pages WHERE page_id = {$page['id']} ORDER BY position ASC";
	$sub_page_set = mysql_query($query);
	confirm_query($sub_page_set); 
	$sub_page = mysql_fetch_array($sub_page_set);
		if ($sub_page !='') {
		 echo '</ul>';
			}
  }
?>	
</ul>
	<!--End Navigation-->

Cups · April 2, 2011, 5:09pm

Good replies.

I think there are 2 ways to go.

You could do just one select that gets everything you need, and you use PHP to iterate through the results.

else;

If you choose to do a nested select, generally frowned upon because connecting to your database is usually one of the slowest parts of your code, well your 3rd select seems rather redundant.

If I address point 1. above.

If your select joined the two tables and presented PHP with all the data it needs to create your nested <ul>s - the first challenge is to get the select working.

Something like this, which is untested might get what you want:
(thankfully you anticipated my next question and posted your table schemas)

Bypass PHP for the moment and paste this into your mysql management tool, PHPMyAdmin or whatever you use.


SELECT p.page name as parent_page 
, s.page_name as subpage_name
FROM pages as p 
LEFT JOIN sub_pages as s
ON p.id = s.page_id
WHERE 
p.id=1
GROUP BY p.id;

To be honest, any SQL guru (a couple of whom to wander into this forum, thankfully) will give likely put me right and give you the full sql statement you need - because they can model this stuff in their heads whereas I need to replicate your database…

Anyhow, what, if anything does that select give you ? ( if your page.id 1 contains sub-pages of course - if not change that number).

ELSE;

In the case of 2. above, I will presume that you use confirm_query() in other places and messing with it will break other parts of your site.

What you want to happen is that if this query brings back zero results you continue, otherwise you build your sub menu


$sub_page_set = mysql_query($query);


if( count($subsetpage) > 0 ){

// now do your submenu, open a ul, 
// add your li's 
// close your /ul

}

Although there are other ways of detecting if you have brought back sub-menu items…

beckerdt · April 2, 2011, 6:47pm

Ok, so I did option one and this is what I got. (I modified your query a little). Now I guess the next step is to figure out a way to output this in PHP so that I can use it for navigation. The way I visioned this navigation working is that I would be able to pull the page name/sub page name, check to see if it is visable using the visible boolean in the database, order the pages/sub pages and output a link that I can use to display the content or a form to edit the page/sub page content.
Query:
mysql> SELECT p.page_name as parent_page
-> , s.page_name as sub_page_name
-> FROM pages as p
-> LEFT JOIN sub_pages as s
-> ON p.id = s.page_id
-> WHERE
-> p.id=3
-> GROUP BY p.id;
±-------------±------------------+
| parent_page | sub_page_name |
±-------------±------------------+
| Photos | Gallery 1 |
±-------------±------------------+
1 row in set (0.05 sec)

mysql>

I just ran the following PHP but I wonder if there is a better kind of loop to use here than the while loop.


<?php
$query = "SELECT p.page_name as parent_page";
 	$query .= ", s.page_name as sub_page_name";
 	$query .= " FROM pages as p";
 	$query .= " LEFT JOIN sub_pages as s";
 	$query .= " ON p.id = s.page_id";
	$query .= " WHERE";
 	$query .= " p.id=3";
 	$query .= " GROUP BY p.id";
	
		$result = mysql_query($query);
		while($title = mysql_fetch_array($result)){
			echo $title['parent_page'] . '<br />';
			}
?>

The output I got from the above code was: Photos (which is one of my page names)

Cups · April 2, 2011, 7:19pm

So how many sub-pages does photos have, more than Gallery 1?

beckerdt · April 2, 2011, 7:50pm

Right now there are 2 pages under photos but could end up being many more. I tried the following code:


<?php
$query = "SELECT p.page_name as parent_page";
 	$query .= ", s.page_name as sub_page_name";
 	$query .= " FROM pages as p";
 	$query .= "	LEFT JOIN sub_pages as s";
 	$query .= "	ON p.id = s.page_id";
	//$query .= " WHERE";
 	//$query .= "	p.id=3";
 	$query .= "	GROUP BY p.id";
		echo '<ul>';
		$result = mysql_query($query);
		while($title = mysql_fetch_array($result)){
			echo '<li>' . $title['parent_page'] . '</li>';
				if($title['sub_page_name'] !=''){
					echo '<ul>';
					echo '<li>' . $title['sub_page_name'] . '</li>';
					echo '</ul>';
					}
			}
		echo '</ul>';
 ?>


The output for this code is:
   Home
   News
   Photos
       Gallery 1
    Videos
    Contact Us
    Blog Pages
        Annette's Page


The output I am trying to get is:
    Home
    News
    Photos
        Gallery 1
        Gallery 2
    Videos
    Contact Us
    Blog Pages
        Annette's Page
        Joshua's Page
        Megan's Page
        Melissa's Page

Cups · April 3, 2011, 11:20am

My bad, you do not need the GROUP BY clause, remove that for the moment, if all works then you can change that to ORDER BY to get the results in the correct order.

Sorry about that, I eventually had to model it my own db to check.