Complex classes like ComponentNavigationTrait 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 ComponentNavigationTrait, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
28 | trait ComponentNavigationTrait { |
||
29 | |||
30 | /** |
||
31 | * AbstractContainer to operate on by default |
||
32 | * |
||
33 | * @var Navigation\AbstractContainer |
||
34 | */ |
||
35 | protected $container; |
||
36 | |||
37 | /** |
||
38 | * The minimum depth a page must have to be included when rendering |
||
39 | * |
||
40 | * @var int |
||
41 | */ |
||
42 | protected $minDepth; |
||
43 | |||
44 | /** |
||
45 | * The maximum depth a page can have to be included when rendering |
||
46 | * |
||
47 | * @var int |
||
48 | */ |
||
49 | protected $maxDepth; |
||
50 | |||
51 | /** |
||
52 | * Indentation string |
||
53 | * |
||
54 | * @var string |
||
55 | */ |
||
56 | protected $indent = ''; |
||
57 | |||
58 | /** |
||
59 | * Whether invisible items should be rendered by this helper |
||
60 | * |
||
61 | * @var bool |
||
62 | */ |
||
63 | protected $renderInvisible = false; |
||
64 | |||
65 | /** |
||
66 | * Sets navigation container the helper operates on by default |
||
67 | * |
||
68 | * Implements {@link HelperInterface::setContainer()}. |
||
69 | * |
||
70 | * @param string|Navigation\AbstractContainer $container Default is null, meaning container will be reset. |
||
71 | * @return self |
||
72 | */ |
||
73 | public function setContainer($container = null) |
||
80 | |||
81 | /** |
||
82 | * Returns the navigation container helper operates on by default |
||
83 | * |
||
84 | * Implements {@link HelperInterface::getContainer()}. |
||
85 | * |
||
86 | * If no container is set, a new container will be instantiated and |
||
87 | * stored in the helper. |
||
88 | * |
||
89 | * @return Navigation\AbstractContainer|Components navigation container |
||
90 | */ |
||
91 | public function getContainer() |
||
99 | |||
100 | /** |
||
101 | * Checks if the helper has a container |
||
102 | * |
||
103 | * Implements {@link HelperInterface::hasContainer()}. |
||
104 | * |
||
105 | * @return bool |
||
106 | */ |
||
107 | public function hasContainer() |
||
111 | |||
112 | /** |
||
113 | * Retrieve whitespace representation of $indent |
||
114 | * |
||
115 | * @param int|string $indent |
||
116 | * @return string |
||
117 | */ |
||
118 | protected function getWhitespace($indent) |
||
126 | |||
127 | /** |
||
128 | * Set the indentation string for using in {@link render()}, optionally a |
||
129 | * number of spaces to indent with |
||
130 | * |
||
131 | * @param string|int $indent |
||
132 | * @return self |
||
133 | */ |
||
134 | public function setIndent($indent) |
||
139 | |||
140 | /** |
||
141 | * Returns indentation |
||
142 | * |
||
143 | * @return string |
||
144 | */ |
||
145 | public function getIndent() |
||
149 | |||
150 | /** |
||
151 | * Sets the maximum depth a page can have to be included when rendering |
||
152 | * |
||
153 | * @param int $maxDepth Default is null, which sets no maximum depth. |
||
154 | * @return self |
||
155 | */ |
||
156 | public function setMaxDepth($maxDepth = null) |
||
166 | |||
167 | /** |
||
168 | * Returns maximum depth a page can have to be included when rendering |
||
169 | * |
||
170 | * @return int|null |
||
171 | */ |
||
172 | public function getMaxDepth() |
||
176 | |||
177 | /** |
||
178 | * Sets the minimum depth a page must have to be included when rendering |
||
179 | * |
||
180 | * @param int $minDepth Default is null, which sets no minimum depth. |
||
181 | * @return self |
||
182 | */ |
||
183 | public function setMinDepth($minDepth = null) |
||
193 | |||
194 | /** |
||
195 | * Returns minimum depth a page must have to be included when rendering |
||
196 | * |
||
197 | * @return int|null |
||
198 | */ |
||
199 | public function getMinDepth() |
||
207 | |||
208 | /** |
||
209 | * Render invisible items? |
||
210 | * |
||
211 | * @param bool $renderInvisible |
||
212 | * @return self |
||
213 | */ |
||
214 | public function setRenderInvisible($renderInvisible = true) |
||
219 | |||
220 | /** |
||
221 | * Return renderInvisible flag |
||
222 | * |
||
223 | * @return bool |
||
224 | */ |
||
225 | public function getRenderInvisible() |
||
229 | |||
230 | /** |
||
231 | * Finds the deepest active page in the given container |
||
232 | * |
||
233 | * @param Navigation\AbstractContainer $container container to search |
||
234 | * @param int|null $minDepth [optional] minimum depth |
||
235 | * required for page to be |
||
236 | * valid. Default is to use |
||
237 | * {@link getMinDepth()}. A |
||
238 | * null value means no minimum |
||
239 | * depth required. |
||
240 | * @param int|null $maxDepth [optional] maximum depth |
||
241 | * a page can have to be |
||
242 | * valid. Default is to use |
||
243 | * {@link getMaxDepth()}. A |
||
244 | * null value means no maximum |
||
245 | * depth required. |
||
246 | * @return array an associative array with |
||
247 | * the values 'depth' and |
||
248 | * 'page', or an empty array |
||
249 | * if not found |
||
250 | */ |
||
251 | public function findActive($container, $minDepth = null, $maxDepth = -1) |
||
304 | |||
305 | /** |
||
306 | * Verifies container and eventually fetches it from service locator if it is a string |
||
307 | * |
||
308 | * @param Navigation\AbstractContainer|string|null $container |
||
309 | * @throws Exception\InvalidArgumentException |
||
310 | */ |
||
311 | protected function parseContainer(&$container = null) |
||
347 | |||
348 | // Iterator filter methods: |
||
349 | |||
350 | /** |
||
351 | * Determines whether a page should be accepted when iterating |
||
352 | * |
||
353 | * Default listener may be 'overridden' by attaching listener to 'isAllowed' |
||
354 | * method. Listener must be 'short circuited' if overriding default ACL |
||
355 | * listener. |
||
356 | * |
||
357 | * Rules: |
||
358 | * - If a page is not visible it is not accepted, unless RenderInvisible has |
||
359 | * been set to true |
||
360 | * - If $useAcl is true (default is true): |
||
361 | * - Page is accepted if listener returns true, otherwise false |
||
362 | * - If page is accepted and $recursive is true, the page |
||
363 | * will not be accepted if it is the descendant of a non-accepted page |
||
364 | * |
||
365 | * @param AbstractPage $page page to check |
||
366 | * @param bool $recursive [optional] if true, page will not be |
||
367 | * accepted if it is the descendant of |
||
368 | * a page that is not accepted. Default |
||
369 | * is true |
||
370 | * |
||
371 | * @return bool Whether page should be accepted |
||
372 | */ |
||
373 | public function accept(AbstractPage $page, $recursive = true) |
||
396 | |||
397 | /** |
||
398 | * Get the service locator. |
||
399 | * |
||
400 | * @abstract |
||
401 | * @return ServiceLocatorInterface |
||
402 | */ |
||
403 | abstract public function getServiceLocator(); |
||
404 | |||
405 | /** |
||
406 | * Returns ACL or null if it isn't set using {@link setAcl()} or |
||
407 | * {@link setDefaultAcl()} |
||
408 | * |
||
409 | * Implements {@link HelperInterface::getAcl()}. |
||
410 | * |
||
411 | * @return Acl\AclInterface|null ACL object or null |
||
412 | */ |
||
413 | abstract public function getAcl(); |
||
414 | |||
415 | /** |
||
416 | * Returns whether ACL should be used |
||
417 | * |
||
418 | * Implements {@link HelperInterface::getUseAcl()}. |
||
419 | * |
||
420 | * @return bool |
||
421 | */ |
||
422 | abstract public function getUseAcl(); |
||
423 | |||
424 | /** |
||
425 | * Returns ACL role to use when iterating pages, or null if it isn't set |
||
426 | * using {@link setRole()} or {@link setDefaultRole()} |
||
427 | * |
||
428 | * Implements {@link HelperInterface::getRole()}. |
||
429 | * |
||
430 | * @return string|Acl\Role\RoleInterface|null |
||
431 | */ |
||
432 | abstract public function getRole(); |
||
433 | |||
434 | } |
||
435 | |||
436 | ?> |
||
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.