Better ways?

[DeanWhitehouse]

So far i have this code to get a students grade from a table, but it has to go through about 4-5 tables to get all the relevant data for getting the grade.

 

Is there a better way to structure my code instead of using lots of $_GET's.

 

My code (cut out db connection, sessions start and html code)

<?php
$sql = mysql_query("SELECT * FROM students WHERE PSN = '".$_SESSION['PSN']."' AND DOB = '".$_SESSION['pass']."' LIMIT 1");//Run a query to list find the student details using his PSN and DOB to locate him/her, limited to 1 to stop any possible errors.
$rows = mysql_fetch_assoc($sql);//get the row with the user details in

echo "<h1>Welcome <a href=\"?logout\" title=\"Logout\">".ucfirst($rows['Name'])." ".ucfirst($rows['Surname'])."</a></h1>";//echo out there first and second names as the logout link
echo "<h3>Login Details</h3>";
echo "<h5>PSN: ".$rows['PSN']."<br>Password: ".$rows['DOB']."</h5>";//echo out there PSN and DOB
?>
<hr align="center">
<?php
$sql = mysql_query("SELECT * FROM student_program WHERE PSN = '".$_SESSION['PSN']."'");//find the programs the student is on
$row = mysql_fetch_assoc($sql);//get the rows of the programs
if(isset($_GET['prog_id']))//if the url contains ?prog_id then store the value as a number inside the var $prog_id
$prog_id = (int) stripslashes($_GET['prog_id']);//get the value and make it a number (stops them using invalid values) and stripslashes
else//if they havent selected a program then use the default one
$prog_id = $row['program_id'];//place the value of the row program_id into the var

if(mysql_num_rows($sql) == 1)//If they are on one program then do 
{
$sql1 = mysql_query("SELECT * FROM program WHERE id = '".$prog_id."' LIMIT 1");//Get the program they are on
$rows = mysql_fetch_assoc($sql1);
echo $rows['name'];//print the program name
}
if(mysql_num_rows($sql) > 1)
{		
?>
<select name="prog_id" onChange="document.location = '?prog_id='+this.value;"><!-- Begin creating the the drop down list -->
<?php

$sql = mysql_query("SELECT * FROM student_program WHERE PSN = '".$_SESSION['PSN']."'");//get the student program details again
while($row = mysql_fetch_assoc($sql))//perform a while loop, to go through each row that mathces the query criteria
{
	$sql1 = mysql_query("SELECT * FROM program WHERE id = '".$row['program_id']."'");//Generate sql query getting program data related to the program_id.
		while($rows = mysql_fetch_assoc($sql1))//run through all the rows for the program data
		{
				?>
				<option onClick="document.location = '?prog_id='+this.value;" value="<?=$rows['id'];?>"><?=$rows['name'];?></option>
				<?php
				//generate the drop down items fill using the program data.
		}
}	
?>
</select>
<?php
$sql = mysql_query("SELECT * FROM program WHERE id = '".$prog_id."' LIMIT 1");//Run query to find the current program being viewed
$rows = mysql_fetch_assoc($sql);//get the row
echo "                    ".$rows['name'];//print the name on screen
}

echo "  -  ";


$length = $rows['length'];//get the length of the course
if($length > 1)
echo $length." years";
else
echo $length." year";
?>
<hr align="center">
<?php
if(isset($_GET['unit_id']))//if the unit id is inputted then
{
$unit_id = (int) stripslashes($_GET['unit_id']);//make sure it is a number
$sql = mysql_query("SELECT * FROM criteria WHERE unit_id = '".$unit_id."'");//Run the query to find the criteria relating to the unit
if(mysql_num_rows($sql) == 0)//if there are no criteria, echo  
	echo "No criteria for this unit.";
else
{
	if(isset($_GET['crit_id']))//If the criteria id is submitted then
	{
		$crit_id = (int) stripslashes($_GET['crit_id']);//Make it a number
		$sql = mysql_query("SELECT * FROM student_grade WHERE criteria_id = '".$crit_id."' AND PSN = '".$_SESSION['PSN']."' AND unit_id = '".$unit_id."' AND program_id = '".$prog_id."' LIMIT 1");//run the query to find the student grade relating to the program,unit and criteria
		if(mysql_num_rows($sql) == 0)//If there is no results then echo
			echo "No grade submitted yet"; 
		else
		{
			$rows = mysql_fetch_assoc($sql);//store the rows 
				echo "Status: ".$rows['grade']."<br>  Comment: ".$rows['comment']."<br>"; //echo comments and grades
		}
	}
	else//if there is no criteria id submitted then
	{
		while($rows = mysql_fetch_assoc($sql))// list criteria for the unit
		{
			echo "<a href=\"?prog_id=".$prog_id."&unit_id=".$unit_id."&crit_id=".$rows['id']."\">".$rows['name']."</a>    ".$rows['description']."<br>";
		}
	}
}
}
else//if there is no unit id submitted
{
for($i = 1;$i <= $length;$i++)
{
	echo "Year ".$i."<hr align=\"center\">";

	$sql = mysql_query("SELECT * FROM units WHERE program_id = '".$prog_id."' AND year = '".$i."'");//get units

	if(mysql_num_rows($sql) == 0)
		echo "No units for this program/year yet";
	else
	{
		while($rows = mysql_fetch_assoc($sql))//list all units related to the program
		{
			echo "   <a href=\"?prog_id=".$prog_id."&unit_id=".$rows['id']."\" title=\"Grades\">".$rows['name']."</a>    ".$rows['description']."<br>";
		}
	}
	echo "<br><br>";
}	
}
?>

[btherl]

I'm not sure how you want to improve your code.  You could use SQL joins in place of multiple queries, that may simplify your code.

 

I also recommend not using variable names like $sql and $sql1.  Instead you can use $sql_programs and $sql_ids, for example.  It makes code clearer and reduces typos.

 

If you want to clean up the usage of $_GET and $_SESSION within your code, you can have a section at the start of the program that gives alternate names for each of those variables you might use later, eg

 

$unit_it = (int) stripslashes($_GET['unit_id']);
$PSN = $_SESSION['PSN'];
etc etc

[haku]

Also, instead of drawing everything from a table row using *, use the specific column names that you want to retrieve instead, even if you want to grab every column, as this will make your queries more efficient.