Page 1 of 1

this code seems impossibly ungainly...

Posted: Wed Nov 23, 2005 10:08 am
by Charles256

Code: Select all

if(isset($kart))
{
	$sql="SELECT * FROM parts WHERE gk_model='$kart'";
	$query=mysql_query($sql);
	$getKartLoc="SELECT * FROM kartloc";
	$getKartQuery=mysql_query($getKartLoc);
	while ($generalLoc=mysql_fetch_object($getKartQuery))
	{
		echo "<b>$generalLoc->kartlocname</b> <br />";
		$getSpecLoc="SELECT * FROM kartspecloc WHERE kartloc='$generalLoc->kartloc'";
		$getSpecQuery=mysql_query($getSpecLoc);
		echo "<ul>";
		while ($specLoc=mysql_fetch_object($getSpecQuery))
		{
			$getImages="SELECT * FROM parts WHERE kartspecloc='$specLoc->speckartloc'";
			$getImagesQuery=mysql_query($getImages);
			$count=mysql_num_rows($getImagesQuery);
			if ($count!=0)
			{
				echo "<li> $specLoc->name </li>";
				while ($images=mysql_fetch_object($getImagesQuery))
				{
					echo "<img src='/images/parts/".$images->part_number.".jpg'>";
				}
			}
		}
		echo "</ul>";
	}
}
any recomendations no how to tidy it up? It just seems so darn bloated to me..course I could be being overly critical...

edit: and as an add on to this page..... do you think since I could be potentially be displaying a <span style='color:blue' title='I&#39;m naughty, are you naughty?'>smurf</span> load of part pictures on this site, should I use AJAX? (the reason I ask is because I'm thinking of adding expand and minimize buttons so they can expand every category, default every category minimized so less screen space is taken up. )

Posted: Wed Nov 23, 2005 10:15 am
by twigletmac
Just from a first glance - you can make your SQL statements much more efficient by selecting only the fields you want to use rather than using * each time...

Mac

Posted: Wed Nov 23, 2005 11:11 am
by trukfixer
I would combine

Code: Select all

$getSpecLoc="SELECT * FROM kartspecloc WHERE kartloc='$generalLoc->kartloc'";
        $getSpecQuery=mysql_query($getSpecLoc);
        echo "<ul>";
        while ($specLoc=mysql_fetch_object($getSpecQuery))
        {
            $getImages="SELECT * FROM parts WHERE kartspecloc='$specLoc->speckartloc'";
those two queries into a sql join instead , and only have to deal with tehone while() loop ..

Posted: Wed Nov 23, 2005 11:15 am
by Charles256
hnm..i'll go look into JOIN..never used it before...thanks for the input people :-D if any more ideas strike ya feel free to speak..afte review I think it's just the long variable names bugging me.