4
\$\begingroup\$

I am going though some PHP arrays exercises on w3resource. I have just completed the following:

  1. Write a PHP script to calculate and display average temperature, five lowest and highest temperatures.

Recorded temperatures : 78, 60, 62, 68, 71, 68, 73, 85, 66, 64, 76, 63, 75, 76, 73, 68, 62, 73, 72, 65, 74, 62, 62, 65, 64, 68, 73, 75, 79, 73

Here is my code:

<?php
    echo "<pre>";
    
    $temperatures = array(78, 60, 62, 68, 71, 68, 73, 85, 66, 64, 76, 63, 75, 76, 73, 68, 62, 73, 72, 65, 74, 62, 62, 65, 64, 68, 73, 75, 79, 73);
    
    function listvalues($value)
    {
        echo "$value, ";
    }
    
    function printAverage($array)
    {
        $total = 0;
        foreach($array as $element)
        {
            $total += $element;
        }
        echo number_format($total / count($array), 1);
    }
    
    echo "Recorded temperatures : ";
    array_walk($temperatures, "listvalues");
    echo "<br>";
    
    echo "Average Temperature is : ";
    printAverage($temperatures);
    echo "<br>";
    
    //sort the temperatures in ascending order for both of the following lists.
    sort($temperatures);
    
    //print the first 5 values
    echo "List of five lowest temperatures : ";
    for($i = 0; $i < 5; $i++)
    {
        echo "$temperatures[$i], ";
    }
    echo "<br>"; 
    
    //print the last 5 values
    echo "List of five highest temperatures : ";       
    for($i = count($temperatures) - 5; $i <= count($temperatures) - 1; $i++)
    {
        echo "$temperatures[$i], ";
    }
    echo "<br>";
    
    echo "</pre>";
?>

And here is its output:

Recorded temperatures : 78, 60, 62, 68, 71, 68, 73, 85, 66, 64, 76, 63, 75, 76, 73, 68, 62, 73, 72, 65, 74, 62, 62, 65, 64, 68, 73, 75, 79, 73, 
Average Temperature is : 69.8
List of seven lowest temperatures : 60, 62, 62, 62, 62, 
List of seven highest temperatures : 76, 76, 78, 79, 85,

(I paid no attention to fixing the commas at the end of each part of the output, for now.)

My questions are:

  • Is this an appropriate use of array_walk()? Is it more appropriate to use a foreach() loop?

  • Is ($total / count($array) the easiest way to find the average of an array, and is that the best way to format it for readability?

  • Can you spot parts of my code which you think could be coded better?

\$\endgroup\$
  • 1
    \$\begingroup\$ Your input is fixed, so you don't need to check for empty arrays. In the general case, you might divide by zero. Just saying. \$\endgroup\$ – coredump May 27 '15 at 18:59
  • 1
    \$\begingroup\$ Interesting how the output says "List of seven...", I would have expected "List of five..." there. :-) \$\endgroup\$ – marcvangend May 27 '15 at 19:38
  • \$\begingroup\$ @marcvangend Indeed; there are many similar mistakes/typos on the other examples. I do my best to ignore them. \$\endgroup\$ – alanbuchanan May 27 '15 at 19:39
8
\$\begingroup\$

There are a number of built-in functions in PHP that would make your code shorter and more direct.

The first is implode. implode takes and optional glue string and an array of values, and joins them together into a single string. So for implode(", ", array(1, "a", 3)), it would return "1, a, 3". This avoids the need to loop over the elements of an array to print them with a separator, and to catch the last case where there should be no separator.

The second is array_sum. This, as its name suggests, takes an array and sums the values. This also avoids an explicit loop.

The third is array_slice. This function will take an array, and return a portion of it based on the given parameters. It can even be used to index in reverse with a negative offset.

Putting it all together, we get the following:

<?php
    echo "<pre>";

    $temperatures = array(78, 60, 62, 68, 71, 68, 73, 85, 66, 64, 76, 63, 75, 76, 73, 68, 62, 73, 72, 65, 74, 62, 62, 65, 64, 68, 73, 75, 79, 73);

    echo "Recorded temperatures : ";
    echo implode(", ", $temperatures);
    echo "<br>";

    echo "Average Temperature is : ";
    echo number_format(array_sum($temperatures) / count($temperatures), 1);
    echo "<br>";

    //sort the temperatures in ascending order for both of the following lists.
    sort($temperatures);

    //print the first 5 values
    echo "List of five lowest temperatures : ";
    echo implode(", ", array_slice($temperatures, 0, 5));
    echo "<br>"; 

    //print the last 5 values
    echo "List of five highest temperatures : ";
    echo implode(", ", array_slice($temperatures, -5, 5));
    echo "<br>";

    echo "</pre>";
?>
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ thanks for this, I mentioned your answer in my blog if that's ok. \$\endgroup\$ – alanbuchanan May 28 '15 at 12:18
  • \$\begingroup\$ @alanbuchanan That's absolutely fine. I'm just glad my answer helped. :) \$\endgroup\$ – cbojar May 28 '15 at 13:04
6
\$\begingroup\$

Is it more appropriate to use a foreach() loop?

You're right. foreach loop would be more appropriate. Use array_walk only when you need to

Apply a user supplied function to every member of an array

This is not the case.

Is ($total / count($array) the easiest way to find the average of an array

Since the average mathematics is total_items / number_items, then yes.

Can you spot parts of my code which you think could be coded better?

The piece of code

for($i = count($temperatures) - 5; $i <= count($temperatures) - 1; $i++)

could be rewritten like this:

for($i = count($temperatures) - 5, $t=count($temperatures) - 1; $i <= $t; $i++)

In this way count($temperatures) - 1 is not performed all the time, only at the beginning of the loop. Documentation: for.

| improve this answer | |
\$\endgroup\$
6
\$\begingroup\$

I don't know PHP, so this won't be a comprehensive review. However, this for statement really jumps out at me:

for($i = count($temperatures) - 5; $i <= count($temperatures) - 1; $i++)

You can write that more simply as:

for($i = count($temperatures) - 5; $i < count($temperatures); $i++)
| improve this answer | |
\$\endgroup\$
  • 3
    \$\begingroup\$ Since count is a function, and it has to iterate over the array to count the values, it is advised to store the result of count inside a variable. Each function has aditional overhead when called. You could do for($length = count($temperatures), $i = $length - 5; $i < $length; $i++);, reducing it to only 1 iteration instead of ... many... \$\endgroup\$ – Ismael Miguel Jan 13 '16 at 18:42

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.