Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like AbstractCalendar often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use AbstractCalendar, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
19 | abstract class AbstractCalendar implements CalendarInterface { |
||
20 | |||
21 | /** |
||
22 | * The units we are dealing with. If no unit ids set the calendar will return |
||
23 | * results for date range and all units within that range. |
||
24 | * |
||
25 | * @var array |
||
26 | */ |
||
27 | protected $units; |
||
28 | |||
29 | |||
30 | /** |
||
31 | * The class that will access the actual event store where event data is held. |
||
32 | * |
||
33 | * @var |
||
34 | */ |
||
35 | protected $store; |
||
36 | |||
37 | /** |
||
38 | * The default value for events. In the event store this is represented by 0 which is then |
||
39 | * replaced by the default value provided in the constructor. |
||
40 | * |
||
41 | * @var |
||
42 | */ |
||
43 | protected $default_value; |
||
44 | |||
45 | |||
46 | /** |
||
47 | * {@inheritdoc} |
||
48 | */ |
||
49 | public function addEvents($events, $granularity) { |
||
67 | |||
68 | /** |
||
69 | * Given a start and end time will retrieve events from the defined store. |
||
70 | * |
||
71 | * If unit_ids where defined it will filter for those unit ids. |
||
72 | * |
||
73 | * @param \DateTime $start_date |
||
74 | * @param \DateTime $end_date |
||
75 | * @return array |
||
76 | */ |
||
77 | public function getEvents(\DateTime $start_date, \DateTime $end_date) { |
||
86 | |||
87 | /** |
||
88 | * Given a start and end time this will return the states units find themselves in for that range. |
||
89 | * |
||
90 | * @param \DateTime $start_date |
||
91 | * @param \DateTime $end_date |
||
92 | * |
||
93 | * @return array |
||
94 | * An array of states keyed by unit |
||
95 | */ |
||
96 | public function getStates(\DateTime $start_date, \DateTime $end_date) { |
||
107 | |||
108 | /** |
||
109 | * Given a date range and a set of valid states it will return all units within the |
||
110 | * set of valid states. |
||
111 | * |
||
112 | * @param \DateTime $start_date |
||
113 | * @param \DateTime $end_date |
||
114 | * @param $valid_states |
||
115 | * @param $constraints |
||
116 | * @param $intersect |
||
117 | * |
||
118 | * @return CalendarResponse |
||
119 | */ |
||
120 | public function getMatchingUnits(\DateTime $start_date, \DateTime $end_date, $valid_states, $constraints = array(), $intersect = FALSE) { |
||
155 | |||
156 | /** |
||
157 | * Provides an itemized array of events keyed by the unit_id and divided by day, |
||
158 | * hour and minute. |
||
159 | * |
||
160 | * @param \DateTime $start_date |
||
161 | * @param \DateTime $end_date |
||
162 | * @param String $granularity |
||
163 | * |
||
164 | * @return array |
||
165 | */ |
||
166 | public function getEventsItemized(\DateTime $start_date, \DateTime $end_date, $granularity = Event::BAT_HOURLY) { |
||
216 | |||
217 | /** |
||
218 | * Helper function that cycles through db results and setups the BAT_DAY itemized array |
||
219 | * |
||
220 | * @param $db_events |
||
221 | * @param $itemized |
||
222 | * @param $unit |
||
223 | * @param $keyed_units |
||
224 | * |
||
225 | * @return array |
||
226 | */ |
||
227 | private function itemizeDays($db_events, $itemized, $unit, $keyed_units) { |
||
248 | |||
249 | /** |
||
250 | * Helper function that cycles through db results and setups the BAT_HOUR itemized array |
||
251 | * @param $db_events |
||
252 | * @param $itemized |
||
253 | * @param $unit |
||
254 | * @param $keyed_units |
||
255 | * |
||
256 | * @return array |
||
257 | */ |
||
258 | private function itemizeHours($db_events, $itemized, $unit, $keyed_units) { |
||
296 | |||
297 | /** |
||
298 | * Helper function that cycles through db results and setups the BAT_MINUTE itemized array |
||
299 | * |
||
300 | * @param $db_events |
||
301 | * @param $itemized |
||
302 | * @param $unit |
||
303 | * @param $keyed_units |
||
304 | * |
||
305 | * @return array |
||
306 | */ |
||
307 | private function itemizeMinutes($db_events, $itemized, $unit, $keyed_units) { |
||
348 | |||
349 | /** |
||
350 | * Given an itemized set of event data it will return an array of Events |
||
351 | * |
||
352 | * @param \DateTime $start_date |
||
353 | * @param \DateTime $end_date |
||
354 | * @param $events |
||
355 | * |
||
356 | * @return array |
||
357 | */ |
||
358 | public function getEventsNormalized(\DateTime $start_date, \DateTime $end_date, $events) { |
||
359 | |||
360 | $normalized_events = array(); |
||
361 | |||
362 | $events_copy = $events; |
||
363 | |||
364 | foreach ($events_copy as $unit_id => $data) { |
||
365 | |||
366 | // Make sure years are sorted |
||
367 | ksort($data[Event::BAT_DAY]); |
||
368 | if (isset($data[Event::BAT_HOUR])) ksort($data[Event::BAT_HOUR]); |
||
369 | if (isset($data[Event::BAT_MINUTE])) ksort($data[Event::BAT_MINUTE]); |
||
370 | |||
371 | // Set up variables to keep track of stuff |
||
372 | $current_value = NULL; |
||
373 | $start_event = new \DateTime(); |
||
374 | $end_event = new \DateTime(); |
||
375 | |||
376 | foreach ($data[Event::BAT_DAY] as $year => $months) { |
||
377 | // Make sure months are in right order |
||
378 | ksort($months); |
||
379 | foreach ($months as $month => $days) { |
||
380 | foreach ($days as $day => $value) { |
||
381 | if ($value == -1) { |
||
382 | // Retrieve hour data |
||
383 | $hour_data = $events[$unit_id][Event::BAT_HOUR][$year][$month][$day]; |
||
384 | ksort($hour_data, SORT_NATURAL); |
||
385 | foreach ($hour_data as $hour => $hour_value) { |
||
386 | if ($hour_value == -1) { |
||
387 | // We are going to need minute values |
||
388 | $minute_data = $events[$unit_id][Event::BAT_MINUTE][$year][$month][$day][$hour]; |
||
389 | ksort($minute_data, SORT_NATURAL); |
||
390 | foreach ($minute_data as $minute => $minute_value) { |
||
391 | if ($current_value === $minute_value) { |
||
392 | // We are still in minutes and going through so add a minute |
||
393 | $end_event->add(new \DateInterval('PT1M')); |
||
394 | } |
||
395 | View Code Duplication | elseif (($current_value != $minute_value) && ($current_value !== NULL)) { |
|
396 | // Value just switched - let us wrap up with current event and start a new one |
||
397 | $normalized_events[$unit_id][] = new Event($start_event, $end_event, $this->getUnit($unit_id), $current_value); |
||
398 | $start_event = clone($end_event->add(new \DateInterval('PT1M'))); |
||
399 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . substr($hour, 1) . ':' . substr($minute,1)); |
||
400 | $current_value = $minute_value; |
||
401 | } |
||
402 | if ($current_value === NULL) { |
||
403 | // We are down to minutes and haven't created and event yet - do one now |
||
404 | $start_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . substr($hour, 1) . ':' . substr($minute,1)); |
||
405 | $end_event = clone($start_event); |
||
406 | } |
||
407 | $current_value = $minute_value; |
||
408 | } |
||
409 | } |
||
410 | elseif ($current_value === $hour_value) { |
||
411 | // We are in hours and can add something |
||
412 | $end_event->add(new \DateInterval('PT1H')); |
||
413 | } |
||
414 | View Code Duplication | elseif (($current_value != $hour_value) && ($current_value !== NULL)) { |
|
415 | // Value just switched - let us wrap up with current event and start a new one |
||
416 | $normalized_events[$unit_id][] = new Event($start_event, $end_event, $this->getUnit($unit_id), $current_value); |
||
417 | // Start event becomes the end event with a minute added |
||
418 | $start_event = clone($end_event->add(new \DateInterval('PT1M'))); |
||
419 | // End event comes the current point in time |
||
420 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . substr($hour, 1) . ':59'); |
||
421 | $current_value = $hour_value; |
||
422 | } |
||
423 | if ($current_value === NULL) { |
||
424 | // Got into hours and still haven't created an event so |
||
425 | $start_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . substr($hour, 1) . ':00'); |
||
426 | // We will be occupying at least this hour so might as well mark it |
||
427 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . substr($hour, 1) . ':59'); |
||
428 | $current_value = $hour_value; |
||
429 | } |
||
430 | } |
||
431 | } |
||
432 | elseif ($current_value === $value) { |
||
433 | // We are adding a whole day so the end event gets moved to the end of the day we are adding |
||
434 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . '23:59'); |
||
435 | } |
||
436 | View Code Duplication | elseif (($current_value !== $value) && ($current_value !== NULL)) { |
|
437 | // Value just switched - let us wrap up with current event and start a new one |
||
438 | $normalized_events[$unit_id][] = new Event($start_event, $end_event, $this->getUnit($unit_id), $current_value); |
||
439 | // Start event becomes the end event with a minute added |
||
440 | $start_event = clone($end_event->add(new \DateInterval('PT1M'))); |
||
441 | // End event becomes the current day which we have not account for yet |
||
442 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . '23:59'); |
||
443 | $current_value = $value; |
||
444 | } |
||
445 | if ($current_value === NULL) { |
||
446 | // We have not created an event yet so let's do it now |
||
447 | $start_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . '00:00'); |
||
448 | $end_event = new \DateTime($year . '-' . $month . '-' . substr($day, 1) . ' ' . '23:59'); |
||
449 | $current_value = $value; |
||
450 | } |
||
451 | } |
||
452 | } |
||
453 | } |
||
454 | |||
455 | // Add the last event in for which there is nothing in the loop to catch it |
||
456 | $normalized_events[$unit_id][] = new Event($start_event, $end_event, $this->getUnit($unit_id), $current_value); |
||
457 | } |
||
458 | |||
459 | // Given the database structure we may get events that are not with the date ranges we were looking for |
||
460 | // We get rid of them here so that the user has a clean result. |
||
461 | foreach ($normalized_events as $unit_id => $events) { |
||
462 | foreach ($events as $key => $event) { |
||
463 | if ($event->overlaps($start_date, $end_date)) { |
||
464 | // Adjust start or end dates of events so everything is within range |
||
465 | if ($event->startsEarlier($start_date)) { |
||
466 | $event->setStartDate($start_date); |
||
467 | } |
||
468 | if ($event->endsLater($end_date)) { |
||
469 | $event->setEndDate($end_date); |
||
470 | } |
||
471 | } |
||
472 | else { |
||
473 | // Event completely not in range so unset it |
||
474 | unset($normalized_events[$unit_id][$key]); |
||
475 | } |
||
476 | } |
||
477 | } |
||
478 | |||
479 | return $normalized_events; |
||
480 | } |
||
481 | |||
482 | /** |
||
483 | * A simple utility function that given an array of datum=>value will group results based on |
||
484 | * those that have the same value. Useful for grouping events based on state. |
||
485 | * |
||
486 | * @param $data |
||
487 | * @param $length |
||
488 | */ |
||
489 | public function groupData($data, $length) { |
||
510 | |||
511 | /** |
||
512 | * Return an array of unit ids from the set of units |
||
513 | * supplied to the Calendar. |
||
514 | * |
||
515 | * @return array |
||
516 | */ |
||
517 | protected function getUnitIds() { |
||
525 | |||
526 | /** |
||
527 | * Return an array of units keyed by unit id |
||
528 | * |
||
529 | * @return array |
||
530 | */ |
||
531 | protected function keyUnitsById() { |
||
539 | |||
540 | /** |
||
541 | * Returns the unit object. |
||
542 | * |
||
543 | * @param $unit_id |
||
544 | * @return Unit |
||
545 | */ |
||
546 | protected function getUnit($unit_id) { |
||
550 | |||
551 | } |
||
552 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.