2
\$\begingroup\$

This is a calendar to layout an array of events/classes (from an API) in calendar format:

       Monday | Tuesday | Wednesday | Thursday | Friday | Saturday | Sunday
time   event  |  event  |    event  |   event  | event  |   event  | event
time   event  |  event  |    event  |   event  | event  |   event  | event
time   event  |  event  |    event  |   event  | event  |   event  | event
time   event  |  event  |    event  |   event  | event  |   event  | event
time   event  |  event  |    event  |   event  | event  |   event  | event
time   event  |  event  |    event  |   event  | event  |   event  | event

This is the function the array of events is fed to:

<?php
function sortClassesByTimeThenDay($mz_classes = array()) {
    $mz_classesByTime = array();

    // Add top level value to each class to store day of week
    for($i=0;$i<count($mz_classes);$i++)
    {
        $mz_classes[$i]['day_num'] = '';
    }

    foreach($mz_classes as &$class)
    {
        /* Create a new array with a key for each time
        and corresponsing value an array of class details 
        for classes at that time. */ 
        $classTime = date_i18n("G.i", strtotime($class['StartDateTime'])); // for numerical sorting
        $display_time = (date_i18n("g:i a", strtotime($class['StartDateTime']))); 
        $classDay = date_i18n("l", strtotime($class['StartDateTime'])); // Weekday name
        $class['day_num'] = date_i18n("N", strtotime($class['StartDateTime'])); // Weekday num 1-7
        if(!empty($mz_classesByTime[$classTime])) {
            $mz_classesByTime[$classTime]['classes'] = array_merge($mz_classesByTime[$classTime]['classes'], array($class));
        } else {
            $mz_classesByTime[$classTime] = array('display_time' => $display_time, 
                                                    'classes' => array($class));
        }
    }
    /* Timeslot keys in new array are not time-sequenced so do so*/
    ksort($mz_classesByTime);
    foreach($mz_classesByTime as $scheduleTime => &$mz_classes)
    {
        /*
        $mz_classes is an array of all classes for given time
        Take each of the class arrays and order it by days 1-7
        */
        usort($mz_classes['classes'], function($a, $b) {
            if(date_i18n("N", strtotime($a['StartDateTime'])) == date_i18n("N", strtotime($b['StartDateTime']))) {
                return 0;
            }
            return $a['StartDateTime'] < $b['StartDateTime'] ? -1 : 1;
        }); 
        $mz_classes['classes'] = week_of_timeslot($mz_classes['classes'], 'day_num');
    }
    return $mz_classesByTime;
}

function week_of_timeslot($array, $indicator){
    /*
    Make a clean array with seven corresponding slots and populate 
    based on indicator (day) for each class. There may be more than
    one event for each day and empty arrays will represent empty time slots.
    */
    $seven_days = array_combine(range(1, 7), array(array(), array(), array(),
                                            array(), array(), array(), array()));
        foreach($seven_days as $key => $value){
            foreach ($array as $class) {
                    if ($class[$indicator] == $key){
                        array_push($seven_days[$key], $class);
                    }
                }
            }
    return $seven_days;
    }
?>

And using HTML Table Class, the calendar is displayed:

<?php
$mz_days = sortClassesByTimeThenDay($mz_days);
$tbl = new HTML_Table('', 'mz-schedule-filter');
        $tbl->addTSection('thead');
        $tbl->addRow();
        // arguments: cell content, class, type (default is 'data' for td, pass 'header' for th)
        // can include associative array of optional additional attributes
        $tbl->addCell('', '', 'header');
        $tbl->addCell('Monday', '', 'header');
        $tbl->addCell('Tuesday', '', 'header');
        $tbl->addCell('Wednesday', '', 'header');
        $tbl->addCell('Thursday', '', 'header');
        $tbl->addCell('Friday', '', 'header');
        $tbl->addCell('Saturday', '', 'header');
        $tbl->addCell('Sunday', '', 'header');

$tbl->addTSection('tbody');
foreach($mz_days as $classDate => $mz_classes)
    {                   
        $tbl->addRow();
        $tbl->addCell($time_of_day, 'hidden', 'data');
        $tbl->addCell($mz_classes['display_time']);
        foreach($mz_classes['classes'] as $key => $classes)
        {
            if(empty($classes)){
                $class_details = '';
                }else{
                $class_details = '';
                $class_separator = (count($classes) > 1) ? '<hr/>' : '';
                foreach($classes as $class){
                        $className = $class['ClassDescription']['Name'];
                        }
                    }
                }
            $tbl->addCell($class_details, 'mz_description_holder');
        }//end foreach mz_classes
    }//end foreach mz_days
$return .= $tbl->display();
?>

I'd like input on styling, comments, syntax, and I'm especially interested in ways to reduce the number of loops being run in sortClassesByTimeThenDay().

It seems like maybe array_map could be used to insert the ordered events into the seven day matrix/array or possibly usort and week_of_timeslot could be combined, but not sure how. At the moment I'm basking in the miracle that I made it work at all.

| improve this question | | | | |
\$\endgroup\$
1
\$\begingroup\$

Prefer foreach when possible

In this loop:

for($i=0;$i<count($mz_classes);$i++)
{
    $mz_classes[$i]['day_num'] = '';
}

The loop index is tedious, and distracting. At a higher level, your actual purpose is to initialize 'day_num' in each element, the array index is only there for technical reasons, it's not really part of your logic.

A foreach loop without an index variable can help focus on your actual purpose:

foreach ($mz_classes as &$mz_class) {
    $mz_class['day_num'] = '';
}

The result is cleaner and shorter too, so overall better in many ways.

Avoid unnecessary operations + limit scope

In this code:

    $display_time = (date_i18n("g:i a", strtotime($class['StartDateTime']))); 
    $classDay = date_i18n("l", strtotime($class['StartDateTime'])); // Weekday name
    $class['day_num'] = date_i18n("N", strtotime($class['StartDateTime'])); // Weekday num 1-7
    if(!empty($mz_classesByTime[$classTime])) {
        $mz_classesByTime[$classTime]['classes'] = array_merge($mz_classesByTime[$classTime]['classes'], array($class));
    } else {
        $mz_classesByTime[$classTime] = array('display_time' => $display_time, 
                                                'classes' => array($class));
    }

$classDay is never used, so it shouldn't be there.

$display_time is only used in the else branch. So it should be created there, for many reasons:

  • Avoid an unnecessary operation when the else branch is not reached
  • Declare variables close to where they are actually used
  • Limit variables to the smallest possible scope: a weak reason in PHP, but a good habit nonetheless

Formatting

This writing style is too compact:

for($i=0;$i<count($mz_classes);$i++)

It would be more readable to follow good spacing conventions in other languages, like this:

for ($i = 0; $i < count($mz_classes); $i++)
| improve this answer | | | | |
\$\endgroup\$
  • \$\begingroup\$ $class['day_num'] = date_i18n("N", strtotime($class['StartDateTime'])); seems to be working within foreach($mz_classes as &$class) now, although the reason I had created the for loop based on stackoverflow.com/questions/25233169/… above it it because it wasn't working previously. Obviously my understanding (and/or testing) is lacking here. \$\endgroup\$ – MikeiLL Jun 28 '15 at 15:40
  • \$\begingroup\$ The trick is the &, to make the loop variable use references instead of values. Otherwise the original list wouldn't be updated, as the loop variable will be a copy instead of the real item in the array. \$\endgroup\$ – janos Jun 28 '15 at 15:42
  • \$\begingroup\$ I actually know about passing by reference with the &, but it's certainly possible I wasn't doing it at the time of the "not working". At any rate, we may have removed an iteration here, which is exciting. \$\endgroup\$ – MikeiLL Jun 28 '15 at 16:14

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.