Welcome to WebmasterWorld Guest from 34.204.36.101

Forum Moderators: coopster & jatar k

Message Too Old, No Replies

Improving a function for loading pages and linking them

     
3:31 pm on Aug 9, 2008 (gmt 0)

New User

10+ Year Member

joined:July 12, 2007
posts: 26
votes: 0


i've written a function for my site to load the pages, the subpages and if its the admin panel, bold the roots of the page

it also bolds the roots, so if you are in news, index + news are bolded as they are the categories your viewing, same for admin pages, even down to the sub sub pages.

now in this function the admin sub-sub pages aren't loaded thats done by another function due to differt layout rules.

all the information is held in mysql.

CID = Category id (id of the main page i.e if index.php has an id of 1, and news is a subcategory news will have a CID of 1)
SID = subcategory id. this is only on the admin pages. this is there to bold the roots. the SID will be equal too the id of the sub category.

Account(category) - Administration(sub category) - Adminpage(sub-sub category)

it works exactly as i need, it'll load active pages if they are active, if they are not it'll load them if the user is an admin.

it'll also only show the pages that user can access unless once again admin.

i just feel that the code could be done in a more elegant way, so if anyone has any ideas please let me know.


function loadPages()
{
require 'err_codes.php';
require 'config.php';
$fname = $_SERVER['PHP_SELF']; // Get file name
$fname = Explode('/', $fname); // Seperate the parts [root], [folder depth] , [file name]
$fname = end($fname); // moves to the end of the array [filename]
$userAccess = getPrivLevel();
$pageCategory = 0;
$isAdminPage = 0;
$count = 0;
$query = 'SELECT cid FROM tablename WHERE url="'. $fname .'"';
$result = mysql_query($query) or die ('Error, query failed. ' . mysql_error()); // use mysql error with custom error
if(mysql_num_rows($result) == 0)
{
$query = 'SELECT cid, sid FROM othertablename WHERE url="'. $fname .'"';
$result = mysql_query($query) or die ('Error, query failed. ' . mysql_error()); // use mysql error with custom error
$isAdminPage = 1;
}
$item = mysql_fetch_array($result);
$pageCategory = $item['cid'];
if($isAdminPage != 0)
{
$isAdminPage = $item['sid'];
}
$query = 'SELECT * FROM firsttablename ORDER BY id';
$result = mysql_query($query) or die ('Error, query failed. ' . mysql_error()); // use mysql error with custom error
$categories = array();
$categoryid = 0;
if(mysql_num_rows($result) == 0)
{
return $err_cate3;
}
else
{
while($row = mysql_fetch_array($result))
{
list($id, $name, $url, $title, $active, $access, $folder) = $row;

if($active == 1 ¦¦ $userAccess >= 10)
{
if($userAccess >= $access)
{
$count += 1;
if($count >= 7)
{
continue;
}
if($url == $fname ¦¦ $pageCategory == $id)
{
$categoryid = $id;
$categories[] = '<div class="menu_entries"><a href="'. $siteurl . $folder . $url . '"><strong>' . $name . ' </strong> </a></div>';
}
else
{
$categories[] = '<div class="menu_entries"><a href="'. $siteurl . $folder . $url . '">' . $name . ' </a></div>';
}

}
else
{
continue;
}
}
else
{
continue;
}
}

}
$categories = implode('', $categories);
echo('<div class="menu">');
echo($categories);
if($categoryid == 0)
{
echo('</div></div>');
return 0; // invalid category
}
$loggedin = 0;
if(isset($_SESSION['lg']))
{
$loggedin = 1;
}
$query = 'SELECT * FROM tablename WHERE cid= ' . $categoryid .' ORDER BY id';
$result = mysql_query($query) or die ('Error, query failed. ' . mysql_error()); // use mysql error with custom error
$subpages = array();
$count = 0;
if($row = mysql_num_rows($result) == 0)
{
echo('</div></div>');
return $err_cate2;
}
else
{
while($row = mysql_fetch_array($result))
{
list($id, $name, $url, $title, $active, $cid, $access, $folder) = $row;
if($active == 1 ¦¦ $userAccess >= 10)
{
if($userAccess >= $access)
{
$count += 1;
if($count >= 7)
{
continue;
}
if($url == 'login.php' && $loggedin == 1)
{
$subpages[] = '<div class="submenu_entries"><a href="'. $siteurl . $folder . 'logout.php' . '">Logout</a></div>';
continue;
}
if($url == $fname ¦¦ $isAdminPage == $id)
{
$subpages[] = '<div class="submenu_entries"><a href="'. $siteurl . $folder . $url . '"><strong>' . $name . '</strong></a></div>';
continue;
}
else
{
$subpages[] = '<div class="submenu_entries"><a href="'. $siteurl . $folder . $url . '">' . $name . '</a></div>';
}
}
else
{
continue;
}
}
else
{
continue;
}
}
}
$subpages = implode('', $subpages);
echo('</div></div>');
echo('<div class="submenu"> ');
echo($subpages);
echo('</div>');
return 1;
}
11:19 pm on Sept 3, 2008 (gmt 0)

Senior Member

joined:Nov 12, 2005
posts:5967
votes: 0


I took a quick look at the code and didn't see anything major. Just make sure that the query variables you are using are clean and don't contain any malicious strings.

As far as making it "neater", and I'm sure the tabbing helps a lot here, but you might want to modularize more of the code into functions for specific tasks.

Have you done any cleaning and reorganizing yourself? How is it coming?

3:43 am on Sept 4, 2008 (gmt 0)

Junior Member

10+ Year Member

joined:June 6, 2005
posts:109
votes: 0


It looks pretty good so far.

As eelixduppy said modularity would make it a bit easier to handle. When I did coding basics at Uni, one of the rules we had to abide by was to never have a function with more than 15 lines of code. It's not a workable practice for real world coding, but it's also not a bad way to get used to being modular.

The two parts that stand out as perfect candidates for being put into their own functions are the sections that create the menu & subpages. They're pretty much idential so you might even be able to fit them (or parts of them) into one function that's called twice. Always good re-use functions where possible to cut down on that amount of code - and work! :)

I would be wary of using list() to split up a $row array from the database. When you start getting into longer pieces of code arrays are a god-send because you can get an idea of the context of the variable straight away... It's easier to tell where $row['id'] comes from than just $id.

7:53 am on Sept 4, 2008 (gmt 0)

New User

10+ Year Member

joined:July 12, 2007
posts: 26
votes: 0


Thanks guys, i will look into turning the code modular, i have done some major updates to the code.


function loadPages($type)
{
require 'err_codes.php';
require 'config.php';

$fname = $_SERVER['PHP_SELF'];
$fname = Explode('/', $fname);
$fname = end($fname);

$userAccess = getPrivLevel();

$loggedin = 0;
if(isset($_SESSION['Lgin']))
{
$loggedin = 1;
}

$query = 'SELECT category FROM tableone WHERE url="'. $fname .'"';
$result = mysql_query($query);

if(mysql_num_rows($result) != 0)
{
$item = mysql_fetch_array($result);
$pageCategory = $item['category'];
}

$query = 'SELECT * FROM tableone ORDER BY id';
$result = mysql_query($query);

$maincategories = array();
$pages = array();
echo('<div class="menu">');
$categoryCount = 0;
if(mysql_num_rows($result) != 0)
{
while($row = mysql_fetch_array($result))
{
list($id, $name, $title, $url, $active, $access, $iscategory, $category, $folder, $viewtype) = $row;

if($userAccess < $access)
{
continue;
}

if($active != 1) // update to allow admins to view the item, set css style for italic
{
continue;
}

if($type == 1)
{

if($iscategory == 1)
{
$categoryCount += 1;
if($categoryCount >= 7)
{
continue;
}
if($fname == $url ¦¦ $pageCategory == $id)
{
$maincategories[] = '<div class="menu_entriesB"><a href="'.$siteurl.$folder.$url.'">'.$name.'</a></div>';
}
else
{
$maincategories[] = '<div class="menu_entries"><a href="'.$siteurl.$folder.$url.'">'.$name.'</a></div>';
}
}
else
{
if($pageCategory == $category && $viewtype == 1)
{
if($url == 'login.php' && $loggedin == 1)
{
$subpages[] = '<div class="submenu_entries"><a href="'.$siteurl.$folder.'logout.php'.'">Logout</a></div>';
continue;
}
if($fname == $url)
{
$pages[] = '<div class="submenu_entriesB"><a href="'.$siteurl.$folder.$url.'">'.$name.'</a></div>';
}
else
{
$pages[] = '<div class="submenu_entries"><a href="'.$siteurl.$folder.$url.'">'.$name.'</a></div>';
}
}
}
}
else if($type == 2)
{
}
}
}
else
{
// no pages
}

$maincategories = implode('', $maincategories);
$pages = implode('', $pages);
echo($maincategories);
echo('</div></div><div class="submenu"> ');
echo($pages);
echo('</div>');

return 1;
}

still alot of work to do on it, and i will heed your warning about the list varibles.

i've removed the need for several seperate tables and amalgamated them into a single table, i've also given the function a varible, to specify what type of menu is it loading, for example:
1 = main menu
2 = sub menu
3 = admin menu

etc etc
as each display either in a different place, or in a different way, i've still got the place the code for the admin menu etc into the function.

Thanks for the advice its been very helpful : o)