Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This PHP script will translate shots on 18 holes to a total under par or E which is 0.

$course = array(0, 4, 5, 4, 3, 5, 3, 4, 4, 4, 4, 4, 4, 4, 3, 5, 3, 4, 5);

if ($totalpar == 0) {
    $score = 'E';
    $played = 0;
} else {
if(!empty($h1)) {
 $h1score = $course[1] - $h1; 
} else {
 $h1score = 0;
 }
if (!empty($h2)) {
 $h2score = $course[2]- $h2; 
} else {
 $h2score = 0;
 }
if (!empty($h3)) {
 $h3score = $course[3] - $h3; 
} else {
 $h3score = 0;
 }
if (!empty($h4)) {
 $h4score = $course[4] - $h4; 
} else {
 $h4score = 0;
 }
if (!empty($h5)) {
 $h5score = $course[5] - $h5; 
} else {
 $h5score = 0;
 }
if (!empty($h6)) {
 $h6score = $course[6] - $h6; 
} else {
 $h6score = 0;
 }
if (!empty($h7)) {
 $h7score = $course[7] - $h7; 
} else {
 $h7score = 0;
 }
if (!empty($h8)) {
 $h8score = $course[8] - $h8; 
} else {
 $h8score = 0;
 }
if (!empty($h9)) {
 $h9score = $course[9] - $h9; 
} else {
 $h9score = 0;
 } 
if (!empty($h10)) {
 $h10score = $course[10] - $h10; 
} else {
 $h10score = 0;
 }
if (!empty($h11)) {
 $h11score = $course[11] - $h11; 
} else {
 $h11score = 0;
 } 
if (!empty($h12)) {
 $h12score = $course[12] - $h12; 
} else {
 $h12score = 0;
 }
if(!empty($h13)) {
 $h13score = $course[13] - $h13; 
} else {
 $h13score = 0;
 }
if(!empty($h14)) {
 $h14score = $course[14] - $h14; 
} else {
 $h14score = 0;
 }
if (!empty($h15)) {
 $h15score = $course[15] - $h15; 
} else {
 $h15score = 0;
 }
 if(!empty($h16)) {
 $h16score = $course[16] - $h16; 
}else {
 $h16score = 0;
 }
if(!empty($h17)) {
 $h17score = $course[17] - $h17; 
}else {
 $h17score = 0;
 }

if(!empty($h18)) {
 $h18score = $course[18] - $h18; 
}else {
 $h18score = 0;
 }
}

$scoresum = ($h1score + $h2score + $h3score + $h4score + $h5score + $h6score + $h7score + $h8score + $h9score + $h10score + $h11score + $h12score + $h13score + $h14score + $h15score + $h16score + $h17score + $h18score) * -1;

if ($scoresum != 0) {
    $score = $scoresum;
}
else {
    $score = 'E';
}
share|improve this question
1  
use an array to store scores as well? –  Bruno Costa May 2 at 21:35

2 Answers 2

up vote 5 down vote accepted

The big thing worth mentioning here is: Use an array for your scores.

The instant you do that, the code size shrinks dramatically.

$course = array(0, 4, 5, 4, 3, 5, 3, 4, 4, 4, 4, 4, 4, 4, 3, 5, 3, 4, 5);
$scores = array();

// $shots is your array of scores.  $shots[1] is the first hole, $shots[2] the second,
// etc.  If you remove the 0 at the beginning of `$course`, then $scores[0] will be
// the first, $scores[1] the second, etc, which is how arrays often look.  But with
// the zero, you can use the hole number as array index -- which might be more useful
// if these numbers are coming from an HTML form.

foreach ($course as $hole => $par) {
    if (!empty($shots[$hole])) {
        $scores[$hole] = $par - $shots[$hole];
    }
}

$scoresum = array_sum($scores);

if ($scoresum != 0) {
    $score = $scoresum;
}
else {
    $score = 'E';
}

But you could do even better than that, if you don't need those intermediate scores.

$course = array(0, 4, 5, 4, 3, 5, 3, 4, 4, 4, 4, 4, 4, 4, 3, 5, 3, 4, 5);
$score = 0;

foreach ($shots as $hole => $total) {
    if (!empty($total)) {
        $score += $course[$hole] - $total;
    }
}

if (empty($score)) $score = 'E';

And because this seems uncommon knowledge, and will simplify your life if these values are indeed from a form, you can get PHP to build your array for you by giving your inputs arrayish names. Like so:

<form>
    <input type="text" name="shots[1]">
    <input type="text" name="shots[2]">
    <input type="text" name="shots[3]">
    <input type="text" name="shots[4]">
    <input type="text" name="shots[5]">
 </form>

(You can even leave out the index, if you don't mind the auto-number indexes PHP does.)

When this form is submitted, $_POST['shots'] will be an array of all the inputs. Do something like:

$shots = array_map('intval', $_POST['shots']);

and you have an array containing the score entered for each hole (or 0 if the entered value didn't make sense as a number).

share|improve this answer

Instead of having 18 variables $h1 ... $h18 and $h1score ... $h18score, you should have arrays and do the calculation in loops.

Also, what is $played ? It is mentioned once, and seems to have no purpose.

share|improve this answer

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.