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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 28, 2015 at 5:13
\$\endgroup\$

1 Answer 1

2
\$\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++)
answered Jun 28, 2015 at 9:58
\$\endgroup\$
3
  • \$\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\$ Commented Jun 28, 2015 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\$ Commented Jun 28, 2015 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\$ Commented Jun 28, 2015 at 16:14

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.