1
\$\begingroup\$

I have the following code that I know can be written in a better way. Basically I query tables and display the results for each student for the homework that has been marked.

<?php
include 'inc/db_con.php';

mysql_select_db("homework", $con);

$resultcourse = mysql_query("SELECT course_code FROM courses WHERE course_active='1' LIMIT 1");
$rowcode = mysql_fetch_assoc($resultcourse);

$result = mysql_query("SELECT homework, total_mark, student, MAX(student_mark), COUNT(total_mark), student_name FROM homeworkone INNER JOIN students on homeworkone.student = students.student_uid  WHERE homeworkone.homework='RegnTheory' && students.student_course='$rowcode[course_code]' GROUP BY student_name ORDER BY student_name ASC");


function countScore($counting)
{
    if($counting == '2'){
        $counted = '&bull;&bull;';
    }
    else if($counting == '3'){
        $counted = '&bull;&bull;&bull;';
    }
    else
       $counted = '&bull;';
    return $counted;
}
?>
<!DOCTYPE html>
 <html lang="en-GB">
  <head>
   <meta charset=utf-8 />
   <title>ACS Homework Results</title>
   <link rel="stylesheet" href="css/reset_css3.css" />
   <link rel="stylesheet" href="css/results.css" />

  </head>
  <body>
   <header>
    <div class="contentwrap">
     <div class="left">
      <h1>ACS Level 3</h1>
      <p>Homework Results <?php echo        $rowcode['course_code']; ?></p>
     </div>
     <div class="right"></div>
    </div>
   </header>
   <div id="wrapper">
    <div id="clearheader"><!-- used to make room for the #header --></div>

    <div class="contentwrap">
     <section>
      <table>
       <tr>
        <th>Name</th>
        <th>1</th>
        <th>2</th>
        <th>3</th>
        <th>4</th>
        <th>5</th>
       </tr>
<?php
 while($row = mysql_fetch_array($result)){ 
  $score = 0;
  $score2 = 0;
  $score3 = 0;
  $score4 = 0;
  $score5 = 0;
  $score6 = 0;
  $score7 = 0;
  $score8 = 0;
  $score8b = 0;
  $score9 = 0;
  $score10 = 0;
  $score11 = 0;
  $score12 = 0;
  $score13 = 0;
  $score14 = 0;
  $score15 = 0;
  $score16 = 0;
  $score17 = 0;
  $score18 = 0;
  $score19 = 0;

 if($row['total_mark']!=0){
  $scorefirst = (100 / $row['total_mark']) * $row['MAX(student_mark)'];
  $score = number_format($scorefirst, 1, '.', '');
  $counter = $row['COUNT(total_mark)'];
  $scoreCounter = countScore($counter);
 }
$stu = $row['student'];

$result2 = mysql_query("SELECT homework, student, total_mark, MAX(student_mark),   COUNT(total_mark) FROM homeworkone WHERE homework='OPtoBCP' && student='$stu'");
while($row2 = mysql_fetch_assoc($result2))
{
 if($row2['total_mark']!=0){     
  $scorefirst2 = (100 / $row2['total_mark']) * $row2['MAX(student_mark)'];
  $score2 = number_format($scorefirst2, 1, '.', ''); }
 $counter = $row2['COUNT(total_mark)'];
 $scoreCounter2 = countScore($counter);
}

$result3 = mysql_query("SELECT homework, student, total_mark, MAX(student_mark), COUNT(total_mark) FROM homeworkone WHERE homework='BCPtoGUN' && student='$stu' GROUP BY student");
while($row3 = mysql_fetch_assoc($result3))
{
 if($row3['total_mark']!=0){
  $scorefirst3 = (100 / $row3['total_mark']) * $row3['MAX(student_mark)'];
  $score3 = number_format($scorefirst3, 1, '.', ''); }}

$result4 = mysql_query("SELECT homework, student, total_mark, MAX(student_mark), COUNT(total_mark) FROM homeworkone WHERE homework='FOOptions' && student='$stu' GROUP BY student");
while($row4 = mysql_fetch_assoc($result4))
{
 if($row4['total_mark']!=0){
  $scorefirst4 = (100 / $row4['total_mark']) * $row4['MAX(student_mark)'];
  $score4 = number_format($scorefirst4, 1, '.', ''); }}

$result5 = mysql_query("SELECT homework, student, total_mark, MAX(student_mark), COUNT(total_mark) FROM homeworkone WHERE homework='CheckMap' && student='$stu' GROUP BY student");
while($row5 = mysql_fetch_assoc($result5))
{
 if($row5['total_mark']!=0){
  $scorefirst5 = (100 / $row5['total_mark']) * $row5['MAX(student_mark)'];
  $score5 = number_format($scorefirst5, 1, '.', ''); }}
?>
       <tr height="45px">
        <td><h3><?php echo $row['student_name']; ?></h3></td>
        <td><?php echo $score; ?>&#37;<br /><?php echo $scoreCounter ?></td>
        <td><?php echo $score2; ?>&#37;<br /><?php echo $scoreCounter2 ?></td>
        <td><?php echo $score3; ?>&#37;</td>
        <td><?php echo $score4; ?>&#37;</td>
        <td><?php echo $score5; ?>&#37;</td>
       </tr>
<?php   }
?>

      </table>
     </section>
    </div>
   </div>
   <div id="results"></div>
  </body>
</html>
\$\endgroup\$
  • \$\begingroup\$ I am thinking that you might want to look at an array for the $score## variables, not sure what all you are doing with those, but it looks like a good spot for an array. it would clean up a little bit of code there, maybe \$\endgroup\$ – Malachi Oct 7 '13 at 19:33
1
\$\begingroup\$

Your queries are only different in the type of homework they look for. Also the processing of the data seems common. The basic way to refactor code like this is to extract it into a method:

function getHomeworkScore($homework, $student)
{
    $score = 0;
    $result = mysql_query("SELECT homework, student, total_mark, MAX(student_mark) as max_mark, COUNT(total_mark) as total_mark_count FROM homeworkone WHERE homework='$homework' && student='$student' GROUP BY student");

     while ($row = mysql_fetch_assoc($result))
     {
         if ($row['total_mark'] != 0)
         {
             $scorefirst = (100 / $row['total_mark']) * $row['max_mark'];
             $score = number_format($scorefirst, 1, '.', ''); 
          }
      }

      return $score;
}

Then you can call it:

$score = getHomeworkScore("OPtoBCP", $stu);
$score2 = getHomeworkScore("BCPtoGUN", $stu);
...

Some notes:

  1. You really want to look into PDO or mysqli otherwise you leave yourself open for a world of pain (SQL injection attacks).

  2. Give your summary column names so you can easily reference them

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.