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 |
||
17 | abstract class AbstractCalendar implements CalendarInterface { |
||
18 | |||
19 | /** |
||
20 | * The units we are dealing with. If no unit ids set the calendar will return |
||
21 | * results for date range and all units within that range. |
||
22 | * |
||
23 | * @var array |
||
24 | */ |
||
25 | protected $units; |
||
26 | |||
27 | |||
28 | /** |
||
29 | * The class that will access the actual event store where event data is held. |
||
30 | * |
||
31 | * @var |
||
32 | */ |
||
33 | protected $store; |
||
34 | |||
35 | /** |
||
36 | * The default value for events. In the event store this is represented by 0 which is then |
||
37 | * replaced by the default value provided in the constructor. |
||
38 | * |
||
39 | * @var |
||
40 | */ |
||
41 | protected $default_value; |
||
42 | |||
43 | |||
44 | /** |
||
45 | * {@inheritdoc} |
||
46 | */ |
||
47 | public function addEvents($events, $granularity) { |
||
48 | |||
49 | $added = TRUE; |
||
50 | |||
51 | foreach ($events as $event) { |
||
52 | // Events save themselves so here we cycle through each and return true if all events |
||
53 | // were saved |
||
54 | |||
55 | $check = $event->saveEvent($this->store, $granularity); |
||
56 | |||
57 | if ($check == FALSE) { |
||
58 | $added = FALSE; |
||
59 | break; |
||
60 | } |
||
61 | } |
||
62 | |||
63 | return $added; |
||
64 | } |
||
65 | |||
66 | /** |
||
67 | * Given a start and end time will retrieve events from the defined store. |
||
68 | * |
||
69 | * If unit_ids where defined it will filter for those unit ids. |
||
70 | * |
||
71 | * @param \DateTime $start_date |
||
72 | * @param \DateTime $end_date |
||
73 | * @return array |
||
74 | */ |
||
75 | public function getEvents(\DateTime $start_date, \DateTime $end_date) { |
||
76 | // We first get events in the itemized format |
||
77 | $itemized_events = $this->getEventsItemized($start_date, $end_date); |
||
78 | |||
79 | // We then normalize those events to create Events that get added to an array |
||
80 | $events = $this->getEventsNormalized($start_date, $end_date, $itemized_events); |
||
81 | |||
82 | return $events; |
||
83 | } |
||
84 | |||
85 | /** |
||
86 | * Given a start and end time this will return the states units find themselves in for that range. |
||
87 | * |
||
88 | * @param \DateTime $start_date |
||
89 | * @param \DateTime $end_date |
||
90 | * @return array |
||
91 | * An array of states keyed by unit |
||
92 | */ |
||
93 | public function getStates(\DateTime $start_date, \DateTime $end_date) { |
||
94 | $events = $this->getEvents($start_date, $end_date); |
||
95 | $states = array(); |
||
96 | foreach ($events as $unit => $unit_events) { |
||
97 | foreach ($unit_events as $event) { |
||
98 | $states[$unit][$event->getValue()] = $event->getValue(); |
||
99 | } |
||
100 | } |
||
101 | |||
102 | return $states; |
||
103 | } |
||
104 | |||
105 | /** |
||
106 | * Given a date range and a set of valid states it will return then units that are withing that |
||
107 | * set of valid states. |
||
108 | * |
||
109 | * @param \DateTime $start_date |
||
110 | * @param \DateTime $end_date |
||
111 | * @param $valid_states |
||
112 | * |
||
113 | * @return CalendarResponse |
||
114 | */ |
||
115 | public function getMatchingUnits(\DateTime $start_date, \DateTime $end_date, $valid_states, $constraints) { |
||
116 | $units = array(); |
||
117 | $response = new CalendarResponse($start_date, $end_date, $valid_states); |
||
118 | $keyed_units = $this->keyUnitsById(); |
||
119 | |||
120 | $states = $this->getStates($start_date, $end_date); |
||
121 | foreach ($states as $unit => $unit_states) { |
||
122 | // Create an array with just the states |
||
123 | $current_states = array_keys($unit_states); |
||
124 | // Compare the current states with the set of valid states |
||
125 | $remaining_states = array_diff($current_states, $valid_states); |
||
126 | if (count($remaining_states) == 0 ) { |
||
127 | // Unit is in a state that is within the set of valid states so add to result set |
||
128 | $units[$unit] = $unit; |
||
129 | $response->addMatch($keyed_units[$unit], CalendarResponse::VALID_STATE); |
||
130 | } |
||
131 | else { |
||
132 | $response->addMiss($keyed_units[$unit], CalendarResponse::INVALID_STATE); |
||
133 | } |
||
134 | |||
135 | $unit_constraints = $keyed_units[$unit]->getConstraints(); |
||
136 | $response->applyConstraints($unit_constraints); |
||
137 | } |
||
138 | |||
139 | $response->applyConstraints($constraints); |
||
140 | |||
141 | return $response; |
||
142 | } |
||
143 | |||
144 | /** |
||
145 | * Provides an itemized array of events keyed by the unit_id and divided by day, |
||
146 | * hour and minute. |
||
147 | * |
||
148 | * @param \DateTime $start_date |
||
149 | * @param \DateTime $end_date |
||
150 | * @param $store |
||
151 | * |
||
152 | * @return array |
||
153 | */ |
||
154 | public function getEventsItemized(\DateTime $start_date, \DateTime $end_date) { |
||
155 | // The final events we will return |
||
156 | $events = array(); |
||
157 | |||
158 | $keyed_units = $this->keyUnitsById(); |
||
159 | |||
160 | $db_events = $this->store->getEventData($start_date, $end_date, array_keys($keyed_units)); |
||
161 | |||
162 | // Create a mock itemized event for the period in question - since event data is either |
||
163 | // in the database or the default value we first create a mock event and then fill it in |
||
164 | // accordingly |
||
165 | $mock_event = new Event($start_date, $end_date, NULL, $this->default_value); |
||
166 | $itemized = $mock_event->itemizeEvent(); |
||
167 | |||
168 | // Cycle through each unit retrieved and provide it with a fully configured itemized mock event |
||
169 | foreach ($db_events as $unit => $event) { |
||
170 | // Add the mock event |
||
171 | $events[$unit] = $itemized; |
||
172 | |||
173 | // Fill in month data coming from the database for our event |
||
174 | foreach ($itemized[Event::BAT_DAY] as $year => $months) { |
||
175 | foreach ($months as $month => $days) { |
||
176 | // Check if month is defined in DB otherwise set to default value |
||
177 | if (isset($db_events[$unit][Event::BAT_DAY][$year][$month])) { |
||
178 | foreach ($days as $day => $value) { |
||
179 | $events[$unit][Event::BAT_DAY][$year][$month][$day] = ((int)$db_events[$unit][Event::BAT_DAY][$year][$month][$day] == 0 ? $keyed_units[$unit]->getDefaultValue() : (int)$db_events[$unit][Event::BAT_DAY][$year][$month][$day]); |
||
180 | } |
||
181 | } |
||
182 | View Code Duplication | else { |
|
|
|||
183 | foreach ($days as $day => $value) { |
||
184 | $events[$unit][Event::BAT_DAY][$year][$month][$day] = $keyed_units[$unit]->getDefaultValue(); |
||
185 | } |
||
186 | } |
||
187 | } |
||
188 | } |
||
189 | |||
190 | |||
191 | // Fill in hour data coming from the database for our event that is represented |
||
192 | // in the mock event |
||
193 | foreach ($itemized[Event::BAT_HOUR] as $year => $months) { |
||
194 | foreach ($months as $month => $days) { |
||
195 | foreach ($days as $day => $hours) { |
||
196 | foreach ($hours as $hour => $value) { |
||
197 | if (isset($db_events[$unit][Event::BAT_HOUR][$year][$month][$day][$hour])) { |
||
198 | $events[$unit][Event::BAT_HOUR][$year][$month]['d' . $day][$hour] = ((int)$db_events[$unit][Event::BAT_DAY][$year][$month][$day][$hour] == 0 ? $keyed_units[$unit]->getDefaultValue() : (int)$db_events[$unit][BAT_DAY][$year][$month][$day][$hour]); |
||
199 | } |
||
200 | View Code Duplication | else { |
|
201 | // If nothing from db - then revert to the defaults |
||
202 | $events[$unit][Event::BAT_HOUR][$year][$month][$day][$hour] = (int)$keyed_units[$unit]->getDefaultValue(); |
||
203 | } |
||
204 | } |
||
205 | } |
||
206 | } |
||
207 | } |
||
208 | |||
209 | // Now fill in hour data coming from the database which the mock event did *not* cater for |
||
210 | // but the mock event |
||
211 | if (isset($db_events[$unit][Event::BAT_HOUR])) { |
||
212 | View Code Duplication | foreach ($db_events[$unit][Event::BAT_HOUR] as $year => $months) { |
|
213 | foreach ($months as $month => $days) { |
||
214 | foreach ($days as $day => $hours) { |
||
215 | foreach ($hours as $hour => $value) { |
||
216 | $events[$unit][Event::BAT_HOUR][$year][$month]['d' . $day][$hour] = ((int) $value == 0 ? $keyed_units[$unit]->getDefaultValue() : (int) $value); |
||
217 | } |
||
218 | } |
||
219 | } |
||
220 | } |
||
221 | } |
||
222 | |||
223 | // Fill in minute data coming from the database for our event that is represented |
||
224 | // in the mock event |
||
225 | foreach ($itemized[Event::BAT_MINUTE] as $year => $months) { |
||
226 | foreach ($months as $month => $days) { |
||
227 | foreach ($days as $day => $hours) { |
||
228 | foreach ($hours as $hour => $minutes) { |
||
229 | foreach ($minutes as $minute => $value) { |
||
230 | if (isset($db_events[$unit][Event::BAT_MINUTE][$year][$month][$day][$hour][$minute])) { |
||
231 | $events[$unit][Event::BAT_MINUTE][$year][$month]['d' .$day]['h'.$hour][$minute] = ((int)$db_events[$unit][BAT_DAY][$year][$month][$day][$hour][$minute] == 0 ? $keyed_units[$unit]->getDefaultValue() : (int)$db_events[$unit][BAT_DAY][$year][$month][$day][$hour][$minute]); |
||
232 | } |
||
233 | View Code Duplication | else { |
|
234 | // If nothing from db - then revert to the defaults |
||
235 | $events[$unit][Event::BAT_MINUTE][$year][$month][$day][$hour][$minute] = (int)$keyed_units[$unit]->getDefaultValue(); |
||
236 | } |
||
237 | } |
||
238 | } |
||
239 | } |
||
240 | } |
||
241 | } |
||
242 | |||
243 | // Now fill in minute data coming from the database which the mock event did *not* cater for |
||
244 | if (isset($db_events[$unit][Event::BAT_MINUTE])) { |
||
245 | foreach ($db_events[$unit][Event::BAT_MINUTE] as $year => $months) { |
||
246 | View Code Duplication | foreach ($months as $month => $days) { |
|
247 | foreach ($days as $day => $hours) { |
||
248 | foreach ($hours as $hour => $minutes) { |
||
249 | foreach ($minutes as $minute => $value) { |
||
250 | $events[$unit][Event::BAT_MINUTE][$year][$month]['d' . $day]['h' . $hour][$minute] = ((int) $value == 0 ? $keyed_units[$unit]->getDefaultValue() : (int) $value); |
||
251 | } |
||
252 | } |
||
253 | } |
||
254 | } |
||
255 | } |
||
256 | } |
||
257 | |||
258 | } |
||
259 | |||
260 | // Check to see if any events came back from the db |
||
261 | if (count($events) == 0) { |
||
262 | // If we don't have any db events add mock events (itemized) |
||
263 | foreach ($keyed_units as $id => $unit) { |
||
264 | $empty_event = new Event($start_date, $end_date, $id, $unit->getDefaultValue()); |
||
265 | $events[$id] = $empty_event->itemizeEvent(); |
||
266 | } |
||
267 | } |
||
268 | |||
269 | return $events; |
||
270 | } |
||
271 | |||
272 | /** |
||
273 | * Given an itemized set of event data it will return an array of Events |
||
274 | * |
||
275 | * @param \DateTime $start_date |
||
276 | * @param \DateTime $end_date |
||
277 | * @param $events |
||
278 | * |
||
279 | * @return array |
||
280 | */ |
||
281 | public function getEventsNormalized(\DateTime $start_date, \DateTime $end_date, $events) { |
||
405 | |||
406 | /** |
||
407 | * A simple utility function that given an array of datum=>value will group results based on |
||
408 | * those that have the same value. Useful for grouping events based on state. |
||
409 | * |
||
410 | * @param $data |
||
411 | * @param $length |
||
412 | */ |
||
413 | public function groupData($data, $length) { |
||
434 | |||
435 | /** |
||
436 | * Return an array of unit ids from the set of units |
||
437 | * supplied to the Calendar. |
||
438 | * |
||
439 | * @return array |
||
440 | */ |
||
441 | public function getUnitIds() { |
||
449 | |||
450 | /** |
||
451 | * Return an array of units keyed by unit id |
||
452 | * |
||
453 | * @return array |
||
454 | */ |
||
455 | public function keyUnitsById() { |
||
463 | |||
464 | } |
||
465 |
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.