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 Collection 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 Collection, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | class Collection extends Paged |
||
24 | { |
||
25 | /** @var string Entity manager instance */ |
||
26 | protected $managerEntity = '\samsoncms\api\query\Generic'; |
||
27 | |||
28 | /** @var array Collection for current filtered material identifiers */ |
||
29 | protected $materialIDs = array(); |
||
30 | |||
31 | /** @var array Collection of navigation filters */ |
||
32 | protected $navigation = array(); |
||
33 | |||
34 | /** @var array Collection of field filters */ |
||
35 | protected $field = array(); |
||
36 | |||
37 | /** @var array Collection of query handlers */ |
||
38 | protected $idHandlers = array(); |
||
39 | |||
40 | /** @var array External material handler and params array */ |
||
41 | protected $entityHandlers = array(); |
||
42 | |||
43 | /** @var array Base material entity handler callbacks array */ |
||
44 | protected $baseEntityHandlers = array(); |
||
45 | |||
46 | /** @var string Collection entities class name */ |
||
47 | protected $entityName = 'samson\cms\CMSMaterial'; |
||
48 | |||
49 | /** |
||
50 | * Generic collection constructor |
||
51 | * |
||
52 | * @param RenderInterface $renderer View render object |
||
53 | * @param QueryInterface $query Query object |
||
54 | */ |
||
55 | public function __construct(RenderInterface $renderer, QueryInterface $query, PagerInterface $pager) |
||
60 | |||
61 | /** |
||
62 | * Render products collection block |
||
63 | * |
||
64 | * @param string $prefix Prefix for view variables |
||
65 | * @param array $restricted Collection of ignored keys |
||
66 | * |
||
67 | * @return array Collection key => value |
||
68 | */ |
||
69 | public function toView($prefix = null, array $restricted = array()) |
||
77 | |||
78 | /** |
||
79 | * Add external identifier filter handler |
||
80 | * |
||
81 | * @param callback $handler |
||
82 | * @param array $params |
||
83 | * |
||
84 | * @return $this Chaining |
||
85 | */ |
||
86 | public function handler($handler, array $params = array()) |
||
93 | |||
94 | /** |
||
95 | * Set external entity handler |
||
96 | * |
||
97 | * @param callback $handler |
||
98 | * @param array $params |
||
99 | * |
||
100 | * @return $this Chaining |
||
101 | */ |
||
102 | public function baseEntityHandler($handler, array $params = array()) |
||
109 | |||
110 | /** |
||
111 | * Set external entity handler |
||
112 | * |
||
113 | * @param callback $handler |
||
114 | * @param array $params |
||
115 | * |
||
116 | * @return $this Chaining |
||
117 | */ |
||
118 | public function entityHandler($handler, array $params = array()) |
||
125 | |||
126 | /** |
||
127 | * Set collection sorter parameters |
||
128 | * |
||
129 | * @param string|integer $field Field identifier or name |
||
130 | * @param string $destination ASC|DESC |
||
131 | * |
||
132 | * @return void |
||
133 | */ |
||
134 | public function sorter($field, $destination = 'ASC') |
||
153 | |||
154 | /** |
||
155 | * Filter collection using navigation entity or collection of them. |
||
156 | * If collection of navigation Url or Ids is passed then this group will be |
||
157 | * applied as single navigation filter to retrieve materials. |
||
158 | * |
||
159 | * @param string|integer|array $navigation Navigation URL or identifier for filtering |
||
160 | * |
||
161 | * @return $this Chaining |
||
162 | */ |
||
163 | public function navigation($navigation) |
||
182 | |||
183 | /** |
||
184 | * Filter collection using additional field entity. |
||
185 | * |
||
186 | * @param string|integer|Field $field Additional field identifier or name |
||
187 | * @param mixed $value Additional field value for filtering |
||
188 | * @param string $relation Additional field relation for filtering |
||
189 | * |
||
190 | * @return $this Chaining |
||
191 | */ |
||
192 | public function field($field, $value, $relation = Relation::EQUAL) |
||
213 | |||
214 | /** |
||
215 | * Filter collection using additional field entity values and LIKE relation. |
||
216 | * If this method is called more then once, it will use materials, previously filtered by this method. |
||
217 | * |
||
218 | * @param string $search Search string |
||
219 | * |
||
220 | * @return $this Chaining |
||
221 | */ |
||
222 | public function search($search) |
||
232 | |||
233 | /** |
||
234 | * Filter collection of numeric field in range from min to max values |
||
235 | * |
||
236 | * @param string|integer|Field $field Additional field identifier or name |
||
237 | * @param integer $minValue Min value for range filter |
||
238 | * @param integer $maxValue Max value for range filter |
||
239 | * |
||
240 | * @return $this Chaining |
||
241 | */ |
||
242 | public function ranged($field, $minValue, $maxValue) |
||
266 | |||
267 | /** |
||
268 | * Try to find additional field record |
||
269 | * |
||
270 | * @param string|integer $field Additional field identifier or name |
||
271 | * |
||
272 | * @return bool True if field record has been found |
||
273 | */ |
||
274 | protected function isFieldObject(&$field) |
||
289 | |||
290 | /** |
||
291 | * Try to get all material identifiers filtered by navigation |
||
292 | * if no navigation filtering is set - nothing will happen. |
||
293 | * |
||
294 | * @param array $filteredIds Collection of filtered material identifiers |
||
295 | * |
||
296 | * @return bool True if ALL navigation filtering succeeded or there was no filtering at all otherwise false |
||
297 | */ |
||
298 | View Code Duplication | protected function applyNavigationFilter(&$filteredIds = array()) |
|
322 | |||
323 | /** |
||
324 | * Try to get all material identifiers filtered by additional field |
||
325 | * if no field filtering is set - nothing will happen. |
||
326 | * |
||
327 | * @param array $filteredIds Collection of filtered material identifiers |
||
328 | * |
||
329 | * @return bool True if ALL field filtering succeeded or there was no filtering at all otherwise false |
||
330 | */ |
||
331 | View Code Duplication | protected function applyFieldFilter(&$filteredIds = array()) |
|
355 | |||
356 | /** |
||
357 | * Try to find all materials which have fields similar to search strings |
||
358 | * |
||
359 | * @param array $filteredIds Collection of filtered material identifiers |
||
360 | * |
||
361 | * @return bool True if ALL field filtering succeeded or there was no filtering at all otherwise false |
||
362 | */ |
||
363 | protected function applySearchFilter(&$filteredIds = array()) |
||
432 | |||
433 | /** |
||
434 | * Apply all possible material filters |
||
435 | * |
||
436 | * @param array $filteredIds Collection of material identifiers |
||
437 | * |
||
438 | * @return bool True if ALL filtering succeeded or there was no filtering at all otherwise false |
||
439 | */ |
||
440 | protected function applyFilter(& $filteredIds = array()) |
||
447 | |||
448 | /** |
||
449 | * Perform material identifiers collection sorting |
||
450 | * |
||
451 | * @param array $materialIDs Variable to return sorted collection |
||
452 | */ |
||
453 | protected function applyFieldSorter(&$materialIDs = array()) |
||
468 | |||
469 | /** |
||
470 | * Perform material own fields sorting |
||
471 | * |
||
472 | * @param array $materialIDs Variable to return sorted collection |
||
473 | * |
||
474 | * @return bool Always true as we are just sorting |
||
475 | */ |
||
476 | protected function applyMaterialSorter(&$materialIDs = array()) |
||
491 | |||
492 | /** |
||
493 | * Call handlers stack |
||
494 | * |
||
495 | * @param array $handlers Collection of callbacks with their parameters |
||
496 | * @param array $params External parameters to pass to callback at first |
||
497 | * |
||
498 | * @return bool True if all handlers succeeded |
||
499 | */ |
||
500 | protected function callHandlers(&$handlers = array(), $params = array()) |
||
513 | |||
514 | /** |
||
515 | * Perform filtering on base material entity |
||
516 | * |
||
517 | * @param array $materialIDs Variable to return sorted collection |
||
518 | */ |
||
519 | protected function applyBaseEntityFilter(&$materialIDs = array()) |
||
546 | |||
547 | /** |
||
548 | * Perform collection database retrieval using set filters |
||
549 | * |
||
550 | * @return $this Chaining |
||
551 | */ |
||
552 | public function fill() |
||
618 | } |
||
619 |
If you access a property on an interface, you most likely code against a concrete implementation of the interface.
Available Fixes
Adding an additional type check:
Changing the type hint: