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.

I'm new to PHP and just wrote a bunch of if statements to display content based on the current time.

Is there a better way of writing the following block of code to easily be maintainable? The only thing that changes in each if statement is the if variables, h4 tag and the $rows variable.

<?php

    date_default_timezone_set("Europe/London");

    $currentTime = date('Hi');
    $breakfastStart = "0200";
    $breakfastEnd = "1129";

    $lunchStart = "1130";
    $lunchEnd = "1559";

    $dinnerStart = "1600";
    $dinnerEnd = "2129";

    $lateStart = "2130";
    $lateEnd = "0159";

?>

<?php if ($currentTime >= $breakfastStart && $currentTime <= $breakfastEnd): ?>
        <?php
            $rows = get_field('breakfast', 41190);
            $row_count = count($rows);
            $i = rand(0, $row_count - 1);
        ?>
        <h4 class="ribbon">Breakfast</h4>
        <h1>
            <?php echo $rows[ $i ]['dish_name']; ?>
        </h1>
        <p>
           <?php echo $rows[ $i ]['dish_description'];  ?>
        </p>
        <h3>
            Served<span><?php echo $rows[ $i ]['time_served'];  ?></span>
        </h3>
<?php endif; ?>

<?php if ($currentTime >= $lunchStart && $currentTime <= $lunchEnd): ?>
        <?php
            $rows = get_field('lunch', 41190);
            $row_count = count($rows);
            $i = rand(0, $row_count - 1);
        ?>
        <h4 class="ribbon">Lunch</h4>
        <h1>
            <?php echo $rows[ $i ]['dish_name']; ?>
        </h1>
        <p>
           <?php echo $rows[ $i ]['dish_description'];  ?>
        </p>
        <h3>
            Served<span><?php echo $rows[ $i ]['time_served'];  ?></span>
        </h3>

<?php endif; ?>

<?php if ($currentTime >= $dinnerStart && $currentTime <= $dinnerEnd): ?>
        <?php
            $rows = get_field('dinner', 41190);
            $row_count = count($rows);
            $i = rand(0, $row_count - 1);
        ?>
        <h4 class="ribbon">Dinner</h4>
        <h1>
            <?php echo $rows[ $i ]['dish_name']; ?>
        </h1>
        <p>
           <?php echo $rows[ $i ]['dish_description'];  ?>
        </p>
        <h3>
            Served<span><?php echo $rows[ $i ]['time_served'];  ?></span>
        </h3>

<?php endif; ?>

<?php if ($currentTime <= $lateStart && $currentTime <= $lateEnd): ?>
        <?php
            $rows = get_field('late_night', 41190);
            $row_count = count($rows);
            $i = rand(0, $row_count - 1);
        ?>
        <h4 class="ribbon">Late Night</h4>
        <h1>
            <?php echo $rows[ $i ]['dish_name']; ?>
        </h1>
        <p>
           <?php echo $rows[ $i ]['dish_description'];  ?>
        </p>
        <h3>
            Served<span><?php echo $rows[ $i ]['time_served'];  ?></span>
        </h3>
<?php endif; ?>
share|improve this question

2 Answers 2

up vote 4 down vote accepted

You are repeating a lot of HTML, which I guess you know could be condensed down, you can also make use of elseif this way it will execute the first TRUE statement and skip the rest.

If there are no matches, for breakfast, lunch, dinner we can assume it must be late night, as there are no more hours left in the day. As this is the case we can just } else { for the last statement.

I have added a check for $row_count as we can never guarantee there will be results returned

<?php

date_default_timezone_set("Europe/London");

$currentTime = date('Hi');
$breakfastStart = "0200";
$breakfastEnd = "1129";

$lunchStart = "1130";
$lunchEnd = "1559";

$dinnerStart = "1600";
$dinnerEnd = "2129";

$lateStart = "2130";
$lateEnd = "0159";

// set up your variables
$rows = $field_key = $field_title = false;

// is it breakfast time?
if( $currentTime >= $breakfastStart && $currentTime <= $breakfastEnd )
{

    // if it is, set the field key and title
    $field_key = 'breakfast';
    $field_title = 'Breakfast';

    // if not, is it lunch time?
} elseif( $currentTime >= $lunchStart && $currentTime <= $lunchEnd )
{

    $field_key = 'lunch';
    $field_title = 'Lunch';

    // if not, is it dinner time?
} elseif( $currentTime >= $dinnerStart && $currentTime <= $dinnerEnd )
{

    $field_key = 'dinner';
    $field_title = 'Dinner';

    // if not, lets assume its late night
} else {

    $field_key = 'late_night';
    $field_title = 'Late Night';

}

$rows = get_field( $field_key , 41190);
$row_count = count($rows);
$i = rand(0, $row_count - 1);

// if we have some rows..
if( $row_count )
{
?>
    <h4 class="ribbon"><?=$field_title?></h4>
    <h1>
        <?php echo $rows[ $i ]['dish_name']; ?>
    </h1>
    <p>
       <?php echo $rows[ $i ]['dish_description'];  ?>
    </p>
    <h3>
        Served<span><?php echo $rows[ $i ]['time_served'];  ?></span>
    </h3>

<?php
}
share|improve this answer
    
Thanks a lot :-) –  user1400803 Jul 18 at 14:55

Welcome to programming. Any time you are repeating code, with a few alterations as per each scenario, you will want to create a function, and pass those variables into the function. This is a very common task, and one you will soon do without realizing.

if ($currentTime >= $breakfastStart && $currentTime <= $breakfastEnd){
    $rows = get_field('breakfast', 41190);
    $timeOfDay = 'Breakfast';
}else if($currentTime >= $lunchStart && $currentTime <= $lunchEnd){
    $rows = get_field('lunch', 41190);
    $timeOfDay = 'Lunch';
}else if($currentTime >= $dinnerStart && $currentTime <= $dinnerEnd){
    $rows = get_field('dinner', 41190);
    $timeOfDay = 'Dinner';
}else if($currentTime <= $lateStart && $currentTime <= $lateEnd){
    $rows = get_field('late_night', 41190);
    $timeOfDay = 'Late Night';
 }

processDish($timeOfDay, $rows);

function processDish($timeOfDay, array $rows)
{
    // get the random row
    $row_count = count($rows);
    $i = rand(0, $row_count - 1);

    // assign the random text
    $dishName = $rows[$i]['dish_name'];
    $dishDescription = $rows[$i]['dish_description'];
    $timeServed = $rows[$i]['time_served'];

    // write the HTML to the browser
    print "<h4 class='ribbon'>$timeOfDay</h4>";
    print "<h1>$dishName</h1>";
    print "<p>$dishDescription</p>";
    print "<h3>Served: $timeServed</h3>";
}
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.