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 TraversalTrait 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 TraversalTrait, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | trait TraversalTrait |
||
16 | { |
||
17 | /** @see CommonTrait::collection() */ |
||
18 | abstract public function collection(); |
||
19 | |||
20 | /** @see CommonTrait::document() */ |
||
21 | abstract public function document(); |
||
22 | |||
23 | /** @see CommonTrait::result() */ |
||
24 | abstract public function result($nodeList); |
||
25 | |||
26 | /** @see ManipulationTrait::inputAsNodeList() */ |
||
27 | abstract public function inputAsNodeList($input); |
||
28 | |||
29 | /** |
||
30 | * @param Traversable|array $nodes |
||
31 | * |
||
32 | * @return NodeList |
||
33 | */ |
||
34 | 132 | public function newNodeList($nodes = null) { |
|
45 | |||
46 | /** |
||
47 | * @param string $selector |
||
48 | * @param string $prefix |
||
49 | * |
||
50 | * @return NodeList |
||
51 | */ |
||
52 | 130 | public function find($selector, $prefix = 'descendant::') { |
|
57 | |||
58 | /** |
||
59 | * @param string $xpath |
||
60 | * |
||
61 | * @return NodeList |
||
62 | */ |
||
63 | 130 | public function findXPath($xpath) { |
|
80 | |||
81 | /** |
||
82 | * @param string|NodeList|\DOMNode|\Closure $input |
||
83 | * @param bool $matchType |
||
84 | * |
||
85 | * @return NodeList |
||
86 | */ |
||
87 | 18 | protected function getNodesMatchingInput($input, $matchType = true) { |
|
120 | |||
121 | /** |
||
122 | * @param string|NodeList|\DOMNode|\Closure $input |
||
123 | * |
||
124 | * @return bool |
||
125 | */ |
||
126 | 15 | public function is($input) { |
|
129 | |||
130 | /** |
||
131 | * @param string|NodeList|\DOMNode|\Closure $input |
||
132 | * |
||
133 | * @return NodeList |
||
134 | */ |
||
135 | 1 | public function not($input) { |
|
138 | |||
139 | /** |
||
140 | * @param string|NodeList|\DOMNode|\Closure $input |
||
141 | * |
||
142 | * @return NodeList |
||
143 | */ |
||
144 | 1 | public function filter($input) { |
|
147 | |||
148 | /** |
||
149 | * @param string|NodeList|\DOMNode|\Closure $input |
||
150 | * |
||
151 | * @return NodeList |
||
152 | */ |
||
153 | 1 | public function has($input) { |
|
185 | |||
186 | /** |
||
187 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
188 | * |
||
189 | * @return \DOMNode|null |
||
190 | */ |
||
191 | public function preceding($selector = null) { |
||
194 | |||
195 | /** |
||
196 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
197 | * |
||
198 | * @return NodeList |
||
199 | */ |
||
200 | 21 | public function precedingAll($selector = null) { |
|
203 | |||
204 | /** |
||
205 | * @param string|NodeList|\DOMNode|\Closure $input |
||
206 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
207 | * |
||
208 | * @return NodeList |
||
209 | */ |
||
210 | 32 | public function precedingUntil($input = null, $selector = null) { |
|
213 | |||
214 | /** |
||
215 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
216 | * |
||
217 | * @return \DOMNode|null |
||
218 | */ |
||
219 | 12 | public function following($selector = null) { |
|
222 | |||
223 | /** |
||
224 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
225 | * |
||
226 | * @return NodeList |
||
227 | */ |
||
228 | 21 | public function followingAll($selector = null) { |
|
231 | |||
232 | /** |
||
233 | * @param string|NodeList|\DOMNode|\Closure $input |
||
234 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
235 | * |
||
236 | * @return NodeList |
||
237 | */ |
||
238 | 25 | public function followingUntil($input = null, $selector = null) { |
|
241 | |||
242 | /** |
||
243 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
244 | * |
||
245 | * @return NodeList |
||
246 | */ |
||
247 | 21 | View Code Duplication | public function siblings($selector = null) { |
258 | |||
259 | /** |
||
260 | * NodeList is only array like. Removing items using foreach() has undesired results. |
||
261 | * |
||
262 | * @return NodeList |
||
263 | */ |
||
264 | 93 | public function children() { |
|
273 | |||
274 | /** |
||
275 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
276 | * |
||
277 | * @return Element|NodeList|null |
||
278 | */ |
||
279 | 51 | public function parent($selector = null) { |
|
284 | |||
285 | /** |
||
286 | * @param int $index |
||
287 | * |
||
288 | * @return \DOMNode|null |
||
289 | */ |
||
290 | 2 | public function eq($index) { |
|
297 | |||
298 | /** |
||
299 | * @param string $selector |
||
300 | * |
||
301 | * @return NodeList |
||
302 | */ |
||
303 | public function parents($selector = null) { |
||
306 | |||
307 | /** |
||
308 | * @param string|NodeList|\DOMNode|\Closure $input |
||
309 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
310 | * |
||
311 | * @return NodeList |
||
312 | */ |
||
313 | public function parentsUntil($input = null, $selector = null) { |
||
316 | |||
317 | /** |
||
318 | * @return \DOMNode |
||
319 | */ |
||
320 | public function intersect() { |
||
339 | |||
340 | /** |
||
341 | * @param string|NodeList|\DOMNode|\Closure $input |
||
342 | * |
||
343 | * @return Element|NodeList|null |
||
344 | */ |
||
345 | 2 | public function closest($input) { |
|
350 | |||
351 | /** |
||
352 | * NodeList is only array like. Removing items using foreach() has undesired results. |
||
353 | * |
||
354 | * @return NodeList |
||
355 | */ |
||
356 | 26 | View Code Duplication | public function contents() { |
367 | |||
368 | /** |
||
369 | * @param string|NodeList|\DOMNode $input |
||
370 | * |
||
371 | * @return NodeList |
||
372 | */ |
||
373 | public function add($input) { |
||
382 | |||
383 | /** @var int */ |
||
384 | private static $MATCH_TYPE_FIRST = 1; |
||
385 | |||
386 | /** @var int */ |
||
387 | private static $MATCH_TYPE_LAST = 2; |
||
388 | |||
389 | /** |
||
390 | * @param \DOMNode $baseNode |
||
391 | * @param string $property |
||
392 | * @param string|NodeList|\DOMNode|\Closure $input |
||
393 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
394 | * @param int $matchType |
||
395 | * |
||
396 | * @return NodeList |
||
397 | */ |
||
398 | 64 | protected function _buildNodeListUntil($baseNode, $property, $input = null, $selector = null, $matchType = null) { |
|
429 | |||
430 | /** |
||
431 | * @param array $nodeLists |
||
432 | * |
||
433 | * @return NodeList |
||
434 | */ |
||
435 | 64 | protected function _uniqueNodes($nodeLists) { |
|
452 | |||
453 | /** |
||
454 | * @param string $property |
||
455 | * @param string|NodeList|\DOMNode|\Closure $input |
||
456 | * @param string|NodeList|\DOMNode|\Closure $selector |
||
457 | * @param int $matchType |
||
458 | * |
||
459 | * @return NodeList |
||
460 | */ |
||
461 | 64 | protected function _walkPathUntil($property, $input = null, $selector = null, $matchType = null) { |
|
470 | } |
This check looks for methods that are used by a trait but not required by it.
To illustrate, let’s look at the following code example
The trait
Idable
provides a methodequalsId
that in turn relies on the methodgetId()
. If this method does not exist on a class mixing in this trait, the method will fail.Adding the
getId()
as an abstract method to the trait will make sure it is available.