Every derived table must have its own alias?

thank mate appreciate it!

One thing I did notice - you’re doing a SELECT * on every occurrence of your tbl_categories. You’ll want to change them so they select only the fields on the table you need for that method.

You’re accessing that table a lot, so limiting what you select there will help some.

hi,

tbl_categories only contains 3 fields catID,catName,cat Parent all of which are used in the query.

ive sussed it :slight_smile:

what was happening was $cat is retreived from the url and used to get the parentID of the category then if that didnt match the toplevel id ‘parentid’ then re run the function, the trouble was that the $cat was alwasys being passed back to the function instead of the new parentID meaning it never reached the top level as it was always looking at the same record hence it was stuck in a loop until it died, so i have to pass the parentID to the function instead of $cat highlighted in red


function parents($pageid,$set,$path,$parentid,$bin,$binName,$age,$ageName,[B][COLOR="Red"]$cat[/COLOR][/B])
{
    if($set == '0')
    {
        $ran = "false";
        echo "<ul>";
    
        if($cat)
        {
            $link = '';
            $seperator = "?";
            if($bin)
            {
                $link = $link.$seperator."bin=".$bin;
                $seperator = "&";
            }
            if($age)
            {
                $link = $link.$seperator."age=".$age;
            }
            if($bin == '' && $age == '')
            {
                $link = $path;
            }
            echo "<li><a href='$link'>Reset</li>";?></a><?php
        }
        $set = '1';
    }
    $pQuery = mysql_query("SELECT * FROM tbl_categories WHERE catID = $cat")or die(mysql_error());
    while($pRow=mysql_fetch_assoc($pQuery))
    {
		$catID = $pRow['catID'];
        $catP = $pRow['catParent'];
        $name = $pRow['catName'];
        $pr = $pRow['catID'];  
        $regexp = "REGEXP '[[:<:]]($pr)[[:>:]]'";
        $divider = "?";
        if($bin)
        {
            $link = $link."&amp;bin=$bin";
            $regexp = $regexp." && filmBinding = '$binName'";
        }
        else
        {
            $regexp = $regexp;
        }
        if($age)
        {
            $link = $link."&amp;age=$age";
            $divider = "&amp;";
            $regexp = $regexp." && filmAgeRating = '$ageName'";
        }
        
		switch($pageid)
		{
			case "dvd";
			$parentitems = mysql_query("SELECT filmDepartment,filmAgeRating,filmBinding FROM tbl_dvds WHERE filmDepartment $regexp")or die(mysql_error());
			break;
		}
        $rows = mysql_num_rows($parentitems);
        $idz[] = $pr;
        $namez[] = $name;
        if($catP != $parentid)
        {
            parents($pageid,$set,$path,$parentid,$bin,$binName,$age,$ageName,[B][COLOR="Red"]$catP[/COLOR][/B]);
        }
        foreach($idz as $idz)
        {
            if($idz != $_GET['cat'])
            {?>
                <li class="nonselect"><a href='?cat=<?php echo $idz?>'><?php echo $namez[0]." (".$rows.")"?></a></li><?php
            }
            else
            {
                echo "<li style='padding-left:15px;'>".$namez[0]." (".$rows.")</li>";
            }
        }
    }
    
    if($ran == "false")
    {
        children($pageid,$bin,$binName,$age,$ageName,$cat);
        $ran = "true";
    }
    if($set == '1' && $ran == "true")
    {
        echo "</ul>";
        $set = '2';
    }    

}

and that solved it, its now working :slight_smile: would still like to hear of any way to improve my script im sure there are many :slight_smile:

A couple things:

  • The one thing that I see that bothers me is the constant building of the links - though I don’t think that will impact your processing time too much. There is a lot of building of link information in the loops which isn’t affected by the loop. If you could move those outside the loops, it would be easier to read, and easier to follow.
  • Is there a reason for all the regex stuff in there? I don’t see a real benefit to it as you’re dealing with an ID and not replacing a title or something.
  • This isn’t a php thing, but I personally would remove the ‘nonselect’ class off all the list items and just make that class style the default list style for those sections. This will make your html smaller, clearer and makes the selected items stand out on quick scan of the source code.

hi,

1/yeah the link building does definately need looking into, maybe build a function? but i will remove them from the loops first :slight_smile:
2/ the regexp stuff is because the filmDepartment field holds several ids which the film is part of ie “810778 1255766 76655 9987665” and im trying to match a single id ie "810778 = would not work like not sure i know “LIKE ‘%810778%’” doesnt work because if there was a number in the field say 77658107786 it would return that item although the id is not matched, maybe just “LIKE ‘810778’” would work? havent tested it for some reason :frowning: but the regexp defo works… will look into the LIKE option
3/thats a good idea would make sence to do this :slight_smile:

thanks mate appreciate the time youve spent looking through my rubble lol

2/ That sounds like your database hasn’t been completely normalized. A filmDepartment table with two columns (filmID and departmentID) would resolve that problem.

ah thats true, at first i didnt think it was necessary but the further i’ve got into my project it seems more and more like a good idea :slight_smile: thanks