Eugene
Reputation: 323
Avoiding the number of function within a class from getting out of hand (not the size of functions)
I have a Calendar class that wraps a bunch of Day classes to represent what is going on in terms of events, exceptions etc... for our schedule related applications.
The problem is as feature list creeps in, and the classes are being used for more and more, the API to the class is growing out of hand. The functions themselves are small since most of algorithms are delegated to other classes, but to keep API simple and not forcing the user to know the underlying structure of the class, I end up wrapping a lot of stuff within functions. Basically it is a bunch of functions calling a method or two in other classes.
The only way I can think about fixing this issue is to extend the Calendar class for every major type of use (like conflict detection).
Due to request for more concrete code here it is:
class Calendar
{
public $start_date;
public $end_date;
/**
* @var Hours_UnitDefault[]
*/
public $unit_defaults = array();
/**
* @var Hours_HoursException[]
*/
public $exceptions = array();
/**
* @var Event[]
*/
public $events = array();
/**
* The underlining data structure of this class.
* In the form of ['date'] => CalendarDay obj
* Why hash table?
* -Convinience/Speed of storage retreaval and organization.
* @var CalendarDay[]
*
*/
public $calendarHashTable = array();
public function __construct()
{
}
public function fetchAll(
$start_date,
$end_date,
array $room_ids,
array $ignore_events = array(),
array $ignore_recurrences = array()
)
{
$this->setDateRange($start_date, $end_date);
$unit_ids = $this->fetchUnitIDs($room_ids);
$this->fetchUnitDefaults($unit_ids);
$semester_ids = array_unique(array_from_key('semester_id', $this->unit_defaults));
$this->fetchExceptions($unit_ids, $semester_ids);
$this->fetchEvents($room_ids, $ignore_events, $ignore_recurrences);
}
/**
* @param Event[] $pending_events
* checks collisions against each pending event and stores the conflicts
*/
public function checkPendingEventsForConflicts(array $pending_events)
{
$this->initHashTableByPendingEvents($pending_events);
$this->hashAllDbData();
$this->selectCorrectUnitDefaults();
$this->calculateCollisionsForPendingEvents($pending_events);
}
/**
* goes through each CalendarDay and makes sure the UnitDefault stored is the one it should use
*/
public function selectCorrectUnitDefaults()
{
foreach ($this->calendarHashTable as $key => $day) {
$day->selectCorrectUnitDefault();
}
}
public function fetchUnitIDs($room_ids)
{
return array_unique(array_from_key('unit_id', RoomUnit::getRoomUnits(array('room_ids' => $room_ids)))); // Directory_));
}
public function fetchUnitDefaults($unit_ids)
{
$this->unit_defaults = Hours_UnitDefault::getUnitDefaults(array('date_range' => array($this->start_date, $this->end_date), 'library_unit_ids' => $unit_ids));
}
public function fetchExceptions($unit_ids, $semester_ids)
{
$this->exceptions = Hours_HoursException::getExceptions(
array(
'with_unit_defaults' => true,
'date_range' => (array(
$this->start_date,
$this->end_date
)),
'semester_unit_id' => $semester_ids[0],
'library_unit_ids' => $unit_ids)
);
}
public function fetchEvents($room_ids, $ignore_events = array(), $ignore_recurrences = array())
{
$this->events = Event::getEvents(array(
'start_date_after' => $this->start_date,
'end_date_before' => $this->end_date . ' 23:59:59',
'room_ids' => $room_ids,
'event_status' => array('approved', 'blocked'),
'not_events' => $ignore_events,
'not_recurrences' => $ignore_recurrences));
}
public function setDateRange($start_date, $end_date)
{
$this->start_date = date("Y-m-j", strtotime($start_date));
$this->end_date = date("Y-m-j", strtotime($end_date));
}
/**
* Gets an array of dates that are relevant to an event type to use as hash keys.
* @static
* @param EventInterface $event
* @return string[] dates
*/
public function getHashDateKeys(EventInterface $event)
{
return ConflictUtility::getDatesBetween2Dates($event->getStartDate(), $event->getEndDate());
}
/**
* Hashes any event type(Event, Exception, UnitDefault) into the hash table.
* @param EventInterface $event
*/
public function hashEvent(EventInterface $event)
{
$keys = $this->getHashDateKeys($event);
foreach ($keys as $key) {
if (!$this->isHashKeySet($key)) {
continue;
}
$day = $this->getCalendarDayByKey($key);
$day->assignEvent($event);
}
}
public function splitEvent(EventInterface $event)
{
//we need to split multi-day spanning days into single days
$split_events = array();
$dates = $this->getHashDateKeys($event);
$dates_count = count($dates);
if ($dates_count > 1) {
foreach ($dates as $date) {
array_push($split_events , clone $event);
}
for ($i = 0; $i setStartDate($dates[$i] . " " . date('g:i:s A', $split_events[$i]->getStartTimeStamp()));
$split_events[$i]->setEndDate($dates[$i] . " " . date('g:i:s A', $split_events[$i]->getEndTimeStamp()));
}
} else { array_push($split_events, $event); }
return $split_events;
}
/**
* Hashes events, exceptions and events into the hash table.
* Assumes the hash table was initialized.
*/
public function hashAllDbData()
{
//we need to split multi-day spanning exceptions into single days
foreach ($this->exceptions as $exception) {
$split_exceptions = $this->splitEvent($exception);
foreach($split_exceptions as $single_exception){
$this->hashEvent($single_exception);
}
}
foreach ($this->events as $event) {
$split_events = $this->splitEvent($event);
foreach($split_events as $single_event){
$this->hashEvent($single_event);
}
}
//splitting unit defaults into days (requires different approach then previous once)
foreach ($this->unit_defaults as $unit_default) {
$unit_default_semester_adapter = new Hours_UnitDefaultEventInterfaceAdopter($unit_default);
$unit_default_semester_adapter->treatAsSemester();
$dates = $this->getHashDateKeys($unit_default_semester_adapter);
foreach ($dates as $date) {
$unit_default_adapter = new Hours_UnitDefaultEventInterfaceAdopter($unit_default);
$unit_default_adapter->treatAsEvent();
$unit_default_adapter->setDate($date);
$this->hashEvent($unit_default_adapter);
}
}
}
/**
* @return CalendarDay
* @param string $key date
*/
public function getCalendarDayByKey($key)
{
return $this->calendarHashTable[$key];
}
/**
*
* @param string $key date
* @return bool
*/
public function isHashKeySet($key)
{
if (isset($this->calendarHashTable[$key])) return true;
else return false;
}
/**
* Inits a hash table key.
* @param string $key date
*/
public function initHashKey($key)
{
$calendar_day = new CalendarDay();
$calendar_day->setDate($key);
$this->calendarHashTable[$key] = $calendar_day;
}
/**
* Sets up hash table based on a date range
* @param string $date_start
* @param string $date_end
*/
public function initHashTableByDateRange($date_start, $date_end)
{
$keys = ConflictUtility::getDatesBetween2Dates($date_start, $date_end);
foreach ($keys as $key) {
$this->initHashKey($key);
}
}
/**
* Sets up hash table based on array of dates
* @param string[] $dates
*/
public function initHashTableByDateArray(array $dates = array())
{
foreach ($dates as $key) {
$this->initHashKey($key);
}
}
/**
* Sets up hash table based on pending events.
* @param Event[] $pending_events
*/
public function initHashTableByPendingEvents(array $pending_events = array())
{
foreach ($pending_events as $pending_event) {
$key = self::getHashDateKey($pending_event);
$this->initHashKey($key);
$calendar_day = $this->getCalendarDayByKey($key);
$calendar_day->assignPendingEvent($pending_event);
}
}
/**
* @param Event[] $modify_pending_events
* @param Event[] $skip_pending_events
*
* deletes pending events that are skipped from the calendar
* substitutes pendings events that are modified from the calendar
*/
public function modifyPendingEvents(array $modify_pending_events, array $skip_pending_events)
{
//replace old one with new one
foreach ($modify_pending_events as $modify_pending_event) {
$key = $this->getHashDateKey($modify_pending_event);
$day = $this->getCalendarDayByKey($key);
$day->emptyPendingEvent();
$day->assignPendingEvent($modify_pending_event);
}
//delete pending events that were skipped
foreach ($skip_pending_events as $skip_pending_event) {
$key = $this->getHashDateKey($skip_pending_event);
$day = $this->getCalendarDayByKey($key);
$day->emptyPendingEvent();
}
}
/**
* @param EventInterface $event
* @return string date
*/
public function getHashDateKey(EventInterface $event)
{
$keys = self::getHashDateKeys($event);
$key = $keys[0];
return $key;
}
/**
* For each pending event, checks even there are collisions in the appropriate date key.
* Behind the scenes it populates the conlficts EventContainer.
* @param Event[] $pending_events
*/
public function calculateCollisionsForPendingEvents(array $pending_events)
{
foreach ($pending_events as $pending_event) {
$key = self::getHashDateKey($pending_event);
$day = $this->getCalendarDayByKey($key);
$day->calculateCollisions();
}
}
/**
* Removes date keys without conflicts.
* Returns a hash table with dates that have conflicts.
* @return array HashTable
*/
public function getConflictingDays()
{
$conflicting_days = array();
foreach ($this->calendarHashTable as $date => $day) {
if ($day->hasConflicts()) {
array_push($conflicting_days, $day);
}
}
return $conflicting_days;
}
public function getConflictFreeDays()
{
$conflict_free_days = array();
foreach ($this->calendarHashTable as $date => $day) {
if (!$day->hasConflicts()) {
array_push($conflict_free_days, $day);
}
}
return $conflict_free_days;
}
/**
* @return Event[]
* returns all of the pending events that are stored on the calendar
*/
public function getPendingEvents()
{
$pending_events = array();
foreach ($this->calendarHashTable as $date => $day) {
if (!$day->isEmptyPendingEvent()) {
array_push($pending_events, $day->getPendingEvent());
}
}
return $pending_events;
}
}
The idea here is that everything is hashed into a [date] => CalendarDay So that all recurrences etc... only span one day. If they span many days, they are broken down into each day it spans.
class CalendarDay
{
/**
* @var EventContainer
*/
public $schedule;
/**
* @var EventContainer
*/
public $conflicts;
public $pending_event;
public $date;
public function __construct()
{
$this->conflicts = new EventContainer();
$this->schedule = new EventContainer();
$this->pending_event = new EventContainer();
}
/**
* Checks whether there are any conflicts in the conflicts container.
* Only relevant if a check for conflicts was performed and conflicts populated.
* @return bool
*/
public function hasConflicts()
{
if ($this->conflicts->isEmpty())
return false;
else
return true;
}
/**
* Figures out if there are collisions between each event type and
* the pending event. In case there is a conflict it appends it to the
* EventContainer conlflicts.
*/
public function calculateCollisions()
{
//if there is no pending events, do nothing.
if ($this->pending_event->isEmptyEvents()) return;
$pending_event = $this->pending_event->getEvent();
if (!$this->schedule->isEmptyEvents()) {
foreach ($this->schedule->getEvents() as $event) {
if ($event->hasConflict($pending_event)) {
$this->conflicts->assignEvent($event);
}
}
}
if (!$this->schedule->isEmptyUnitDefault()) {
$default_hours = $this->schedule->getUnitDefault();
if ($default_hours->hasConflict($pending_event)) {
$this->conflicts->assignEvent($default_hours);
}
}
if (!$this->schedule->isEmptyException()) {
$exception = $this->schedule->getException();
if ($exception->hasConflict($pending_event)) {
$this->conflicts->assignEvent($exception);
}
}
}
/**
* Assigns event the to EventContainer schedule
* @param EventInterface $event
*/
public function assignEvent(EventInterface $event)
{
$this->schedule->assignEvent($event);
}
public function assignPendingEvent(EventInterface $event)
{
$this->pending_event->assignEvent($event);
}
/**
* Basic idea behind the algorithm:
* ********************************
* Open Period(START)
* |open -> between opening and first event
* Closed Period(START)
* Closed Period(END)
* |open -> between events
* Closed Period(START)
* Closed Period(END)
* |open -> between closing and last event
* Open Period(END)
* ********************************
*
* @return TimePeriod[]
* returns empty array if closed
*/
public function getAvailableHours() //todo:make sure data is valid before this algorithm happens
{
$available_hours = array();
$open_hours = $this->getOpenTimePeriod();
//means its closed this day (cases: closed all day exception, closed on that day in unit default)
if ($open_hours->isEmpty()) return $available_hours; //which is an empty array
$closed_time_periods = $this->getClosedTimePeriods($open_hours);
$events_size = count($closed_time_periods);
$hours_start = $open_hours->getStartTimeStamp();
$hours_end = $open_hours->getEndTimeStamp();
//if events are empty, the available hours are the open hours
if (empty($closed_time_periods)) {
$available = new TimePeriod();
$available->setStartTimeStamp($hours_start);
$available->setEndTimeStamp($hours_end);
array_push($available_hours, $available);
return $available_hours;
} else {
//takes care of available hours between opening hour and first event
$first_event = $closed_time_periods[0];
$first_available = new TimePeriod();
$first_available->setStartTimeStamp($hours_start);
$first_available->setEndTimeStamp($first_event->getStartTimeStamp());
array_push($available_hours, $first_available);
//takes care of available hours in between events if more then one
if ($events_size > 1) {
for ($i = 0; $i setStartTimeStamp($event->getEndTimeStamp());
$available->setEndTimeStamp($next_event->getStartTimeStamp());
array_push($available_hours, $available);
}
}
//takes care of available hours between closing hour and last event
$last_event = $closed_time_periods[$events_size - 1];
$last_available = new TimePeriod();
$last_available->setStartTimeStamp($last_event->getEndTimeStamp());
$last_available->setEndTimeStamp($hours_end);
array_push($available_hours, $last_available);
}
//filter out available hours less then 30m //todo: ask slava if its 30m
foreach ($available_hours as $i => $available) {
$minutes = ($available->getEndTimeStamp() - $available->getStartTimeStamp()) / 60;
if ($minutes schedule->isEmptyUnitDefault()) {
$unit_default = $this->schedule->getUnitDefault();
$open_hours->setStartTimeStamp($unit_default->getStartTimeStamp());
$open_hours->setEndTimeStamp($unit_default->getEndTimeStamp());
$open_hours->setSource($unit_default);
}
//if there is an exception for open hours, replace default hours
else if (!$this->schedule->isEmptyException()) {
$exception = $this->schedule->getException();
if ($exception->is_open) {
$open_hours->setStartTimeStamp($exception->getStartTimeStamp());
$open_hours->setEndTimeStamp($exception->getEndTimeStamp());
$open_hours->setSource($exception);
}
}
return $open_hours;
}
/**
* @param TimePeriod $open_hours
* @return TimePeriod[]
*/
public function getClosedTimePeriods(TimePeriod $open_hours)
{
$closed_hours = array();
//first take care of events and treat them as closed time periods
$events = $this->schedule->getEvents();
foreach ($events as $event) {
$closed_period = new TimePeriod();
//add 10 minutes before and after for events //todo:: possibly account for events that go right after each other to avoid 20m gap instead of 10m
$padding = 0;
if ($event->event_status != 'blocked') { $padding = 10 * 60; }
$closed_period->setSource($event);
//don't need the date
$closed_period->setStartTimeStamp($event->getStartTimeStamp() - $padding);
$closed_period->setEndTimeStamp($event->getEndTimeStamp() + $padding);
array_push($closed_hours, $closed_period);
}
//take care of closed exception if any and treat it as closed time period
if (!$this->schedule->isEmptyException()) {
$exception = $this->schedule->getException();
if (!$exception->is_open) {
$closed_period = new TimePeriod();
$closed_period->setSource($exception);
$closed_period_start = $exception->getStartTimeStamp();
$closed_period_end = $exception->getEndTimeStamp();
//open hours generally don't have date as part of time stamp so we need to add it for future comparisons with exception
$open_period_start = $open_hours->getStartTimeStamp();
$open_period_end = $open_hours->getEndTimeStamp();
//now we need to take care of cases when closed exception starts before or after opening hours.
//why? to simplify further algorithms so that all closed time period fall within open time period.
//if closed exception starts before open -> trunkate
if ($closed_period_start $open_period_end) {
$closed_period_end = $open_period_end;
}
$closed_period->setStartTimeStamp($closed_period_start);
$closed_period->setEndTimeStamp($closed_period_end);
array_push($closed_hours, $closed_period);
}
if (!function_exists('cmp')) {
function cmp($a, $b)
{
if ($a->getStartTimeStamp() == $b->getStartTimeStamp()) {
return 0;
}
return ($a->getStartTimeStamp() getStartTimeStamp()) ? -1 : 1;
}
}
usort($closed_hours, 'cmp');
}
return $closed_hours;
}
public function selectCorrectUnitDefault()
{
if (!$this->schedule->isEmptyException()) {
$exception = $this->schedule->getException();
if ($exception->is_open) { //if we have exception for open hours, get rid of defaults
$this->schedule->emptyUnitDefaults();
return;
}
if (!$exception->is_open) { //check for closed exception that last all day //todo:: think about how to do this in available_periods section
if ($exception->getStartTimeStamp('only_time') == strtotime('12:00:00 AM') && $exception->getEndTimeStamp('only_time') == strtotime('11:59:00 PM')) {
$this->schedule->emptyUnitDefaults();
return;
}
}
}
$unit_defaults = $this->schedule->getUnitDefaults();
//discard library hours if there are multiple unit defaults b/c we are going to use a different one.
if (count($unit_defaults) > 1) {
foreach ($unit_defaults as $i => $unit_default) {
if ($unit_default->getName() == 'Library') unset($unit_defaults[$i]);
}
$unit_defaults = array_values($unit_defaults);
//take the more restrictive unit default
$last_unit_default = $unit_defaults[0];
foreach ($unit_defaults as $unit_default) {
if ($unit_default->getStartTimeStamp() > $last_unit_default->getStartTimeStamp()) $last_unit_default = $unit_default;
}
$unit_defaults[0] = $last_unit_default;
}
$unit_default = $unit_defaults[0]; //take whatever remains as unit default
$this->schedule->emptyUnitDefaults();
$this->schedule->assignEvent($unit_default);
}
/**
* Sets date to correspond to hash table date key for convinience
* @param string $date
*/
public function setDate($date)
{
$this->date = $date;
$this->schedule->setDate($date);
$this->conflicts->setDate($date);
$this->pending_event->setDate($date);
}
public function getDate($format = "Y-m-d")
{
return date($format, strtotime($this->date));
}
public function getAllConflicts()
{
return $this->conflicts->getAllEvents();
}
public function getConflictCount()
{
return count($this->getAllConflicts());
}
public function emptyPendingEvent()
{
$this->pending_event->emptyEvents();
}
public function emptyConflicts()
{
$this->conflicts->emptyAll();
}
public function isEmptyPendingEvent()
{
return $this->pending_event->isEmptyEvents();
}
public function getPendingEvent()
{
return $this->pending_event->getEvent();
}
}
A container CalendarDay uses for storage.
class EventContainer {
/**
* @var Hours_UnitDefaultInterfaceAdopter[]
*/
private $unit_defaults = array();
/**
* @var Hours_HoursException
*/
private $exception;
/**
* @var Event[]
*/
private $events = array();
/**
* Assigns event type to the appropriate variable by checking its class.
* UnitDefault are saved as UnitDefaultEventInterfaceAdopter for code reuse.
* @param EventInterface|Hours_UnitDefaultEventInterfaceAdopter|Hours_HoursException|Event $event
*/
public function assignEvent(EventInterface $event) {
// echo "assigning: ".get_class($event) . "
";
switch (get_class($event)) {
case 'Event':
array_push($this->events, $event);
break;
case 'Hours_HoursException':
$this->exception = $event;
break;
case 'Hours_UnitDefaultEventInterfaceAdopter':
array_push($this->unit_defaults, $event);
break;
default:
die('Invalid class type argument supplied to the DaySchedule->assign() function');
}
}
/**
* Checks whether all of the event types arrays are empty.
* @return bool
*/
public function isEmpty(){
if (empty($this->unit_defaults) && empty($this->exception) && empty($this->events)) return true;
else return false;
}
/**
*
* @return bool
*/
public function isEmptyException(){
return empty($this->exception);
}
/**
*
* @return bool
*/
public function isEmptyEvents(){
return empty($this->events);
}
/**
*
* @return bool
*/
public function isEmptyUnitDefault(){
return empty($this->unit_defaults);
}
/**
* @return Hours_HoursException
*/
public function getException(){
return $this->exception;
}
/*
* @return Hours_UnitDefaultInterfaceAdopter
*/
public function getUnitDefault(){
return $this->unit_defaults[0];
}
/*
* @return Hours_UnitDefaultInterfaceAdopter[]
*/
public function getUnitDefaults(){
return $this->unit_defaults;
}
/*
* @return Event[]
*/
public function getEvents(){
return $this->events;
}
public function getEvent(){
return $this->events[0];
}
public function setDate($date){
$this->date = $date;
}
public function getAllEvents(){
$events = array();
if(!$this->isEmptyEvents()){
$events = $this->events;
}
if(!$this->isEmptyException()){
array_push($events, $this->exception);
}
if(!$this->isEmptyUnitDefault()){
array_push($events, $this->getUnitDefault());
}
return $events;
}
public function emptyEvents(){
$this->events = array();
}
public function emptyUnitDefaults(){
$this->unit_defaults = array();
}
public function emptyException(){
$this->exception = null;
}
public function emptyAll(){
$this->emptyEvents();
$this->emptyException();
$this->emptyUnitDefaults();
}
}
Interface that is used everywhere.
interface EventInterface
{
public function getStartTimeStamp();
public function getEndTimeStamp();
public function getStartDate();
public function getEndDate();
public function hasConflict(EventInterface $pending_event);
public function getName();
public function getDetails();
}
Now the problem is all the conflict detection etc... Require me to have seprate EventContainer objects composed in CalendarDay to store pending events etc... As features grow I find myself adding too much stuff to it.
Any critique is welcome.
I am here to learn. Anyone here that took the time to read: thanks ahead of time!
Upvotes: 0
Views: 165
Answers (1)
XiaoChuan Yu
Reputation: 4011
Disclaimer: I actually have zero PHP experience (currently).
Correct me if I'm wrong but it seems your Calendar class divides a time interval into CalendarDays, then inside each CalendarDay you deal with events separately and then re-combine the results in Calendar again. If this is the case, then I think you should be following his advice above. You should not be having this intermediate step of splitting time interval into days and process events in each day separately. Instead, you should deal with them in more generic way across multiple days and do the math to map them to days implicitly, not splitting them explicitly into separate objects.
Upvotes: 0
Related Questions
- php structuring classes properly to provide the required functionalities
- Splitting a large PHP class
- Group functions within class
- can we define lot of methods in php class?
- Encapsulate functionalities into classes
- Using OOP for combining lots of functions (sort of namespace) in PHP?
- Improving class structure in PHP instead of using multiple inheritance
- A group of related PHP functions: what is the most acceptable way to organize them?
- php oop, class structure with many class
- OOP design question - Avoid repeating code in similar classes?