Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

This script updates a mySQL data with golf scores and calculates how many strokes under the player is in real time and is working. Just looking for a better way to make it cleaner and compact. Any ideas would be appreciated!

<?php
// Get values from form 
$uid=$_POST['uid'];
$name=$_POST['name'];
$course=$_POST['course'];
$h01=$_POST['h01'];
$h02=$_POST['h02'];
$h03=$_POST['h03'];
$h04=$_POST['h04'];
$h05=$_POST['h05'];
$h06=$_POST['h06'];
$h07=$_POST['h07'];
$h08=$_POST['h08'];
$h09=$_POST['h09'];
$h10=$_POST['h10'];
$h11=$_POST['h11'];
$h12=$_POST['h12'];
$h13=$_POST['h13'];
$h14=$_POST['h14'];
$h15=$_POST['h15'];
$h16=$_POST['h16'];
$h17=$_POST['h17'];
$h18=$_POST['h18'];
$played=$_POST['played'];

$n01=4;
$n02=5;
$n03=4;
$n04=3;
$n05=5;
$n06=3;
$n07=4;
$n08=4;
$n09=4;
$n10=4;
$n11=4;
$n12=4;
$n13=4;
$n14=3;
$n15=5;
$n16=3;
$n17=4;
$n18=5;

$d01=4;
$d02=5;
$d03=4;
$d04=3;
$d05=5;
$d06=3;
$d07=4;
$d08=4;
$d09=4;
$d10=4;
$d11=4;
$d12=4;
$d13=4;
$d14=3;
$d15=5;
$d16=3;
$d17=4;
$d18=5;

$s01=4;
$s02=5;
$s03=4;
$s04=3;
$s05=5;
$s06=3;
$s07=4;
$s08=4;
$s09=4;
$s10=4;
$s11=4;
$s12=4;
$s13=4;
$s14=3;
$s15=5;
$s16=3;
$s17=4;
$s18=5;

if ($course = 'Augusta')
{   
    if( $h01 > 0) {
        $s01 = $h01 - $n01;
    }else{
        $s01 = 0;
    }
    if( $h02 > 0) {
        $s02 = $h02 - $n02;
    }else{
        $s02 = 0;
    }
    if( $h03 > 0) {
        $s03 = $h03 - $n03;
    }else{
        $s03 = 0;
    }
    if( $h04 > 0) {
        $s04 = $h04 - $n04;
    }else{
        $s04 = 0;
    }
    if( $h05 > 0) {
        $s05 = $h05 - $n05;
    }else{
        $s05 = 0;
    }
    if( $h06 > 0) {
        $s06 = $h06 - $n06;
    }else{
        $s06 = 0;
    }
    if( $h07 > 0) {
        $s07 = $h07 - $n07;
    }else{
        $s07 = 0;
    }
    if( $h08 > 0) {
        $s08 = $h08 - $n08;
    }else{
        $s08 = 0;
    }
    if( $h09 > 0) {
        $s09 = $h09 - $n09;
    }else{
        $s09 = 0;
    }
    if( $h10 > 0) {
        $s10 = $h10 - $n10;
    }else{
        $s10 = 0;
    }
    if( $h11 > 0) {
        $s11 = $h11 - $n11;
    }else{
        $s11 = 0;
    }
    if( $h12 > 0) {
        $s12 = $h12 - $n12;
    }else{
        $s12 = 0;
    }
    if( $h13 > 0) {
        $s13 = $h13 - $n13;
    }else{
        $s13 = 0;
    }
    if( $h14 > 0) {
        $s14 = $h14 - $n14;
    }else{
        $s14 = 0;
    }
    if( $h15 > 0) {
        $s15 = $h15 - $n15;
    }else{
        $s15 = 0;
    }
    if( $h16 > 0) {
        $s16 = $h16 - $n16;
    }else{
        $s16 = 0;
    }
    if( $h17 > 0) {
        $s17 = $h17 - $n17;
    }else{
        $s17 = 0;
    }
    if( $h18 > 0) {
        $s18 = $h18 - $n18;
    }else{
        $s18 = 0;
    }
}
else 
{
        if( $h01 > 0) {
            $s01 = $h01 - $d01;
        }else{
            $s01 = 0;
        }
        if( $h02 > 0) {
            $s02 = $h02 - $d02;
        }else{
            $s02 = 0;
        }
        if( $h03 > 0) {
            $s03 = $h03 - $d03;
        }else{
            $s03 = 0;
        }
        if( $h04 > 0) {
            $s04 = $h04 - $d04;
        }else{
            $s04 = 0;
        }
        if( $h05 > 0) {
            $s05 = $h05 - $d05;
        }else{
            $s05 = 0;
        }
        if( $h06 > 0) {
            $s06 = $h06 - $d06;
        }else{
            $s06 = 0;
        }
        if( $h07 > 0) {
            $s07 = $h07 - $d07;
        }else{
            $s07 = 0;
        }
        if( $h08 > 0) {
            $s08 = $h08 - $d08;
        }else{
            $s08 = 0;
        }
        if( $h09 > 0) {
            $s09 = $h09 - $d09;
        }else{
            $s09 = 0;
        }
        if( $h10 > 0) {
            $s10 = $h10 - $d10;
        }else{
            $s10 = 0;
        }
        if( $h11 > 0) {
            $s11 = $h11 - $d11;
        }else{
            $s11 = 0;
        }
        if( $h12 > 0) {
            $s12 = $h12 - $d12;
        }else{
            $s12 = 0;
        }
        if( $h13 > 0) {
            $s13 = $h13 - $d13;
        }else{
            $s13 = 0;
        }
        if( $h14 > 0) {
            $s14 = $h14 - $d14;
        }else{
            $s14 = 0;
        }
        if( $h15 > 0) {
            $s15 = $h15 - $d15;
        }else{
            $s15 = 0;
        }
        if( $h16 > 0) {
            $s16 = $h16 - $d16;
        }else{
            $s16 = 0;
        }
        if( $h17 > 0) {
            $s17 = $h17 - $d17;
        }else{
            $s17 = 0;
        }
        if( $h18 > 0) {
            $s18 = $h18 - $d18;
        }else{
            $s18 = 0;
        }
}

$score = $s01 + $s02 + $s03 + $s04 + $s05 + $s06 + $s07 + $s08 + $s09 + $s10 + $s11 + $s12 + $s13 + $s14 + $s15 + $s16 + $s17 + $s18; 

// update data in mysql database
$sql="UPDATE $tbl_name
        SET name='$name',
            course='$course',
            h01='$h01',
            h02='$h02',
            h03='$h03',
            h04='$h04',
            h05='$h05',
            h06='$h06',
            h07='$h07',
            h08='$h08',
            h09='$h09',
            h10='$h10',
            h11='$h11',
            h12='$h12',
            h13='$h13',
            h14='$h14',
            h15='$h15', 
            h16='$h16', 
            h17='$h17', 
            h18='$h18', 
            played='$played',
            score='$score' 
        WHERE uid='$uid'";

$result=mysql_query($sql);

// if successfully updated.
if($result){
// echo "<script>window.location = 'index.php'</script>";
echo "<h1>UPDATED - <a href='index.php'>Back to Admin Page</a></h1>";
}

else {
echo "<h1>ERROR</h1>";
}

?>
share|improve this question

1 Answer

up vote 2 down vote

There are a number of ways to improve this code:

  1. You really need to use arrays for this.
  2. $d and $n are identical so I don't see any need for the if ($course = 'Augusta') condition.
  3. Use msqli methods instead of mysql, as they've been deprecated.
  4. Use prepared statements to guard against sql injection.

Try this:

function _getName($i) { return 'h' . str_pad($i, '0', 2, STR_PAD_LEFT); }
function _getPost($n) { return $_POST[$n]; }
function _getSqlParam($n) { return $n.'=?'; }
$hNames =  array_map(_getName, range(1, 18));
$h = array_map(_getPost, $hNames);
$played=$_POST['played'];
$n = array(4, 5, 4, 3, 5, 3, 4, 4, 4, 4, 4, 4, 4, 3, 5, 3, 4, 5);
$s = $n; // copy array

for($i = 0; $i < count($h); ++$i) {
    if( $h[$i] > 0) {
        $s[$i] = $h[$i] - $n[$i];
    } else {
        $s[$i] = 0;
    }
}

$sqlSetClause = implode(',', array_map(_getSqlParam, $hNames));
$sql="UPDATE $tbl_name 
    SET name=?,course=?,played=?,score=?,$sqlSetClause
    WHERE uid=?";
$mysqli = new mysqli("localhost", "my_user", "my_password", "world");
if ($stmt = $mysqli->prepare($sql)) {
    $allParams = array_merge(array($name, $course, $played, $score), $h, array($uid));
    $stmt->bind_param(str_repeat('s', count($allParams)), $allParams);
    $result = $stmt->execute();

    if($result) {
        // echo "<script>window.location = 'index.php'</script>";
        echo "<h1>UPDATED - <a href='index.php'>Back to Admin Page</a></h1>";
    }
}
share|improve this answer
Thanks for the good info. – Conor Apr 16 at 15:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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