4
\$\begingroup\$

I'm new to PHP and had to put together a function that loads a random image from an array (it works fine). Here's what I came up with. Could it be improved? Any feedback would be appreciated.

<?php
function displayRandomPhotoArea() {
    $photoAreas = array("imageURL1", "imageURL2", "imageURL3", "imageURL4", "imageURL5");

    $randomNumber = rand(0, (count($photoAreas) - 1));

    echo '<img src="' . $photoAreas[$randomNumber] . '" width="725" height="194">';
}

// Display a random image here
displayRandomPhotoArea();
?>
\$\endgroup\$
  • 1
    \$\begingroup\$ I don't think this can be improved that much. You could make it more generic by passing the url-array as a parameter and return a random image-tag instead of directly displaying it. \$\endgroup\$ – Abbas Feb 28 '13 at 8:52
3
\$\begingroup\$
<?php
function displayRandomPhotoArea() {
    $photoAreas = array("imageURL1", "imageURL2", "imageURL3", "imageURL4", "imageURL5");

    $randomNumber = rand(0, (count($photoAreas) - 1));

You can use directly the array_rand instead of this combination of rand and count, but this comment on the documentation says that it is not too random, so you can keep your code like this. More generally, always try to make use of the standard library in PHP which offers a number of useful functions.

    echo '<img src="' . $photoAreas[$randomNumber] . '" width="725" height="194">';

You should return the string here, instead of echoing it. This is better design for a number of reasons:

  • This separates the concern of echoing with the concern of building the string
  • You don't want to restrict yourself to simply echoing it, you might want to use it afterwards in other ways
  • You should start echoing only when you're certain that you're not going to issue a redirect to another page. Even if this is not an issue today, it will be someday, and you should be prepared.

Regarding separation of concern, it is good practice to separate logic (here choosing a random image) and presentation (displaying the image). Web frameworks use variations of a design pattern named MVC which you might want to learn about, at least to separate your views from the rest of your code. This might not be needed if your code is very short, but it's still an useful tool.

\$\endgroup\$
3
\$\begingroup\$

Try to use array_rand function, code will look cleaner:

<?php
function displayRandomPhotoArea() 
{
    $photoAreas = array("imageURL1", "imageURL2", "imageURL3", "imageURL4", "imageURL5");

    $randomNumber = array_rand($photoAreas);
    $randomImage = $photoAreas[$randomNumber];

    echo "<img src=\"$randomImage\" width=\"725\" height=\"194\">";
}

displayRandomPhotoArea();
?>
\$\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.