Problem
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.
Solution
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
elsebranch 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++)