I wrote a feature for my website today. Let’s say you have two volunteer shifts. One takes place from 9AM to 1PM. The other takes place from 11AM to 3PM.

Now let’s say you are a volunteer picking shifts on my website. I want the website to throw an error if there are more than 65 minutes of overlap between shifts (it’s a waste to sign up for a shift and then not be able to work a lot of it). So in the above example, that would throw an error that the shifts overlap too much (by 120 minutes).

Anyway, I finished scripting it. But for something so simple, the code ended up being surprisingly complicated. That’s a red flag for me that I didn’t code it well.

I would like to run the code by you guys and see if I can get some advice on 1) how to make it more efficient, and 2) if the way I chose to group things (one giant helper function) is good, or if you recommend something different.

This is written in PHP using the CodeIgniter framework. CodeIgniter has lots of different places to put things, so I’m still getting the hang of it. Right now this is a “helper”, but it could also be a “library”, broken up into more methods, etc.

Screenshot

controllers/Volunteers.php

check_for_overlapping_shifts($ this->data['list_of_shifts']); 

helpers/mv_form_validation_helper.php

function check_for_overlapping_shifts($ list_of_shifts) {     $ obj = &get_instance();     $ obj->benchmark->mark('check_for_overlapping_shifts_start');      assert(is_array($ list_of_shifts));     foreach ( $ list_of_shifts as $ shift )     {         assert(isset($ shift['shift_id']));         assert(isset($ shift['shift_start_time']));         assert(isset($ shift['shift_end_time']));     }      if ( $ list_of_shifts === array() || ! isset($ _POST['shift_id']) )     {         return;     }      // Make sure we are only checking shifts that exist. The form validation library will handle non-existent shifts for the user. We can just ignore them here to prevent them throwing errors.     $ shift_ids_to_check = array();     foreach ( $ list_of_shifts as $ shift )     {         foreach ($ _POST['shift_id'] as $ post_key => $ value )         {             if ( $ post_key == $ shift['shift_id'] )             {                 array_push($ shift_ids_to_check, $ shift['shift_id']);             }         }     }      // This data came from SQL, so it doesn't have level 1 keys. Rebuild the array with level 1 keys so that we don't have to use the processor intensive functions like sql_search_result_array_by_key1_value1() and sql_search_result_array_by_key1_and_return_key2() when searching for values in the array.     $ list_of_shifts = sql_rebuild_array_with_better_keys($ list_of_shifts, 'shift_id');      if ( count($ shift_ids_to_check) >= 2 )     {         $ overlapping_shifts = array();         foreach ( $ shift_ids_to_check as $ outer_shift_id )         {             foreach ( $ shift_ids_to_check as $ inner_shift_id )             {                 if (                     $ outer_shift_id != $ inner_shift_id &&                     // The very complex check below is to prevent overlapping shifts from being tallied twice. Let's say the shift_id's to be checked are 7 and 13. The way these nested loops are set up, 7-13 and 13-7 have exactly the same data, but will be logged twice. This conditional prevents that.                     ! (                         sql_search_result_array_by_key1_value1($ overlapping_shifts, 'outer_shift_id', $ inner_shift_id) &&                         sql_search_result_array_by_key1_value1($ overlapping_shifts, 'inner_shift_id', $ outer_shift_id)                     )                 )                 {                     $ outer_shift_start_time = strtotime($ list_of_shifts[$ outer_shift_id]['shift_start_time']);                      $ outer_shift_end_time = strtotime($ list_of_shifts[$ outer_shift_id]['shift_end_time']);                      $ inner_shift_start_time = strtotime($ list_of_shifts[$ inner_shift_id]['shift_start_time']);                      $ inner_shift_end_time = strtotime($ list_of_shifts[$ inner_shift_id]['shift_end_time']);                      // https://stackoverflow.com/questions/325933/determine-whether-two-date-ranges-overlap                     if ( $ outer_shift_start_time <= $ inner_shift_end_time && $ outer_shift_end_time >= $ inner_shift_start_time )                     {                         if ( $ outer_shift_end_time > $ inner_shift_start_time )                         {                             $ minutes_of_overlap = ($ outer_shift_end_time - $ inner_shift_start_time) / 60;                         }                         else                         {                             $ minutes_of_overlap = ($ inner_shift_end_time - $ outer_shift_start_time) / 60;                         }                          array_push($ overlapping_shifts, array(                             'outer_shift_id' => $ outer_shift_id,                             'inner_shift_id' => $ inner_shift_id,                             'outer_shift_name' => $ list_of_shifts[$ outer_shift_id]['shift_name'],                             'inner_shift_name' => $ list_of_shifts[$ inner_shift_id]['shift_name'],                             'outer_shift_start_time' => $ outer_shift_start_time,                             'inner_shift_start_time' => $ inner_shift_start_time,                             'outer_shift_end_time' => $ outer_shift_end_time,                             'inner_shift_end_time' => $ inner_shift_end_time,                             'minutes_of_overlap' => $ minutes_of_overlap                         ));                     }                 }             }         }          foreach ( $ overlapping_shifts as $ key => $ shift )         {             if ( $ shift['minutes_of_overlap'] > MAX_MINUTES_OF_SHIFT_OVERLAP_FOR_PUBLIC )             {                 $ obj->form_validation->add_error_message('Volunteer Shifts', $ shift['outer_shift_name'] . ' and ' . $ shift['inner_shift_name'] . ' overlap by more than ' . MAX_MINUTES_OF_SHIFT_OVERLAP_FOR_PUBLIC . " minutes. Please choose shifts that don't overlap.");                  break;             }         }     }      $ obj->benchmark->mark('check_for_overlapping_shifts_end'); } 

helpers/mv_misc_helper.php

/* input must be an array in the format     [0] =>         'group_id' => 5         'group_name' => 'Test Group 2'     [1] =>         'group_id' => 2         'group_name' => 'Test Group 1' */ // In the above example, this function will take group_id's value and make that the [0] [1] key. That way the array can be searched by the key field natively, instead of using a time consuming search function. function sql_rebuild_array_with_better_keys($ array, $ new_key_field_name) {     $ new_array = array();      foreach ( $ array as $ key => $ level2 )     {         // Key field chosen must contain unique keys. Throw error if not unique.         assert(! isset($ new_array[ $ level2[$ new_key_field_name] ]));          // Key field must exist. Throw error if not set.         assert(isset($ level2[$ new_key_field_name]));          $ new_array[ $ level2[$ new_key_field_name] ] = $ level2;     }      return $ new_array; }  /* input must be an array in the format     [0] =>         'group_id' => 5         'group_name' => 'Test Group 2'     [1] =>         'group_id' => 2         'group_name' => 'Test Group 1' */ // use === and !== in booleans to avoid having a '0' value act like a NULL value function sql_search_result_array_by_key1_value1($ array, $ search_key, $ search_value) {     foreach ( $ array as $ key => $ level2 )     {         if ( $ level2[$ search_key] == $ search_value )         {             return TRUE;         }     }      return NULL; } 

config/constants.php

define('MAX_MINUTES_OF_SHIFT_OVERLAP_FOR_PUBLIC', 65); 

libraries/MY_Form_validation.php

<?php  if ( ! defined('BASEPATH')) exit('No direct script access allowed');  class MY_Form_validation extends CI_Form_validation {     // This both displays an error message, and prevents form_validation from evaluating to TRUE. Use this to make form_validation fail.     public function add_error_message($ field, $ message)     {         if ( ! isset($ this->_error_array[$ field]))         {             $ this->_error_array[$ field] = $ message;         }     } }