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 ApiCollection 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 ApiCollection, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class ApiCollection extends AbstractLazyCollection implements Selectable |
||
15 | { |
||
16 | /** @var ApiEntityManager */ |
||
17 | private $manager; |
||
18 | /** @var ApiMetadata */ |
||
19 | private $metadata; |
||
20 | /** @var object */ |
||
21 | private $owner; |
||
22 | /** @var array */ |
||
23 | private $association; |
||
24 | /** @var bool */ |
||
25 | private $isDirty = false; |
||
26 | /** @var array */ |
||
27 | private $snapshot = []; |
||
28 | /** @var string */ |
||
29 | private $backRefFieldName; |
||
30 | |||
31 | /** |
||
32 | * ApiCollection constructor. |
||
33 | * |
||
34 | * @param ApiEntityManager $manager |
||
35 | * @param ApiMetadata $class |
||
36 | * @param Collection $collection |
||
37 | */ |
||
38 | 11 | public function __construct( |
|
48 | |||
49 | /** |
||
50 | * @return boolean |
||
51 | */ |
||
52 | 2 | public function isDirty() |
|
56 | |||
57 | 2 | public function unwrap() |
|
61 | |||
62 | /** |
||
63 | * INTERNAL: |
||
64 | * Tells this collection to take a snapshot of its current state. |
||
65 | * |
||
66 | * @return void |
||
67 | */ |
||
68 | 4 | public function takeSnapshot() |
|
73 | |||
74 | 2 | public function getMapping() |
|
78 | |||
79 | /** |
||
80 | * @return object |
||
81 | */ |
||
82 | 3 | public function getOwner() |
|
86 | |||
87 | /** |
||
88 | * @param object $owner |
||
89 | * @param array $assoc |
||
90 | */ |
||
91 | 11 | public function setOwner($owner, array $assoc) |
|
97 | |||
98 | /** |
||
99 | * @param bool $dirty |
||
100 | */ |
||
101 | 2 | public function setDirty($dirty) |
|
105 | |||
106 | /** |
||
107 | * Initializes the collection by loading its contents from the database |
||
108 | * if the collection is not yet initialized. |
||
109 | * |
||
110 | * @return void |
||
111 | */ |
||
112 | 3 | public function initialize() |
|
120 | |||
121 | /** |
||
122 | * @return ApiMetadata |
||
123 | */ |
||
124 | public function getMetadata() |
||
128 | |||
129 | /** |
||
130 | * INTERNAL: |
||
131 | * Adds an element to a collection during hydration. This will automatically |
||
132 | * complete bidirectional associations in the case of a one-to-many association. |
||
133 | * |
||
134 | * @param mixed $element The element to add. |
||
135 | * |
||
136 | * @return void |
||
137 | */ |
||
138 | public function hydrateAdd($element) |
||
156 | |||
157 | /** |
||
158 | * INTERNAL: |
||
159 | * Sets a keyed element in the collection during hydration. |
||
160 | * |
||
161 | * @param mixed $key The key to set. |
||
162 | * @param mixed $element The element to set. |
||
163 | * |
||
164 | * @return void |
||
165 | */ |
||
166 | public function hydrateSet($key, $element) |
||
179 | |||
180 | /** |
||
181 | * INTERNAL: |
||
182 | * Returns the last snapshot of the elements in the collection. |
||
183 | * |
||
184 | * @return array The last snapshot of the elements. |
||
185 | */ |
||
186 | public function getSnapshot() |
||
190 | |||
191 | 9 | public function setInitialized($state) |
|
195 | |||
196 | /** |
||
197 | * INTERNAL: |
||
198 | * getDeleteDiff |
||
199 | * |
||
200 | * @return array |
||
201 | */ |
||
202 | View Code Duplication | public function getDeleteDiff() |
|
212 | |||
213 | /** |
||
214 | * INTERNAL: |
||
215 | * getInsertDiff |
||
216 | * |
||
217 | * @return array |
||
218 | */ |
||
219 | View Code Duplication | public function getInsertDiff() |
|
229 | |||
230 | /** |
||
231 | * {@inheritdoc} |
||
232 | */ |
||
233 | View Code Duplication | public function remove($key) |
|
250 | |||
251 | /** |
||
252 | * {@inheritdoc} |
||
253 | */ |
||
254 | 1 | public function removeElement($element) |
|
282 | |||
283 | /** |
||
284 | * {@inheritdoc} |
||
285 | */ |
||
286 | View Code Duplication | public function containsKey($key) |
|
298 | |||
299 | /** |
||
300 | * {@inheritdoc} |
||
301 | */ |
||
302 | 1 | View Code Duplication | public function contains($element) |
312 | |||
313 | /** |
||
314 | * {@inheritdoc} |
||
315 | */ |
||
316 | public function get($key) |
||
333 | |||
334 | /** |
||
335 | * {@inheritdoc} |
||
336 | */ |
||
337 | 1 | public function count() |
|
347 | |||
348 | /** |
||
349 | * {@inheritdoc} |
||
350 | */ |
||
351 | View Code Duplication | public function set($key, $value) |
|
359 | |||
360 | /** |
||
361 | * {@inheritdoc} |
||
362 | */ |
||
363 | 2 | View Code Duplication | public function add($value) |
373 | |||
374 | /** |
||
375 | * {@inheritdoc} |
||
376 | */ |
||
377 | public function offsetExists($offset) |
||
381 | |||
382 | /** |
||
383 | * {@inheritdoc} |
||
384 | */ |
||
385 | public function offsetGet($offset) |
||
389 | /* ArrayAccess implementation */ |
||
390 | |||
391 | /** |
||
392 | * {@inheritdoc} |
||
393 | */ |
||
394 | public function offsetSet($offset, $value) |
||
402 | |||
403 | /** |
||
404 | * {@inheritdoc} |
||
405 | */ |
||
406 | public function offsetUnset($offset) |
||
410 | |||
411 | /** |
||
412 | * {@inheritdoc} |
||
413 | */ |
||
414 | 2 | public function isEmpty() |
|
418 | |||
419 | /** |
||
420 | * {@inheritdoc} |
||
421 | */ |
||
422 | public function clear() |
||
447 | |||
448 | /** |
||
449 | * Called by PHP when this collection is serialized. Ensures that only the |
||
450 | * elements are properly serialized. |
||
451 | * |
||
452 | * Internal note: Tried to implement Serializable first but that did not work well |
||
453 | * with circular references. This solution seems simpler and works well. |
||
454 | * |
||
455 | * @return array |
||
456 | */ |
||
457 | public function __sleep() |
||
461 | |||
462 | /** |
||
463 | * Extracts a slice of $length elements starting at position $offset from the Collection. |
||
464 | * |
||
465 | * If $length is null it returns all elements from $offset to the end of the Collection. |
||
466 | * Keys have to be preserved by this method. Calling this method will only return the |
||
467 | * selected slice and NOT change the elements contained in the collection slice is called on. |
||
468 | * |
||
469 | * @param int $offset |
||
470 | * @param int|null $length |
||
471 | * |
||
472 | * @return array |
||
473 | */ |
||
474 | View Code Duplication | public function slice($offset, $length = null) |
|
484 | |||
485 | /** |
||
486 | * Cleans up internal state of cloned persistent collection. |
||
487 | * |
||
488 | * The following problems have to be prevented: |
||
489 | * 1. Added entities are added to old PC |
||
490 | * 2. New collection is not dirty, if reused on other entity nothing |
||
491 | * changes. |
||
492 | * 3. Snapshot leads to invalid diffs being generated. |
||
493 | * 4. Lazy loading grabs entities from old owner object. |
||
494 | * 5. New collection is connected to old owner and leads to duplicate keys. |
||
495 | * |
||
496 | * @return void |
||
497 | */ |
||
498 | public function __clone() |
||
508 | |||
509 | /** |
||
510 | * Selects all elements from a selectable that match the expression and |
||
511 | * return a new collection containing these elements. |
||
512 | * |
||
513 | * @param \Doctrine\Common\Collections\Criteria $criteria |
||
514 | * |
||
515 | * @return Collection |
||
516 | * |
||
517 | * @throws \RuntimeException |
||
518 | */ |
||
519 | public function matching(Criteria $criteria) |
||
538 | |||
539 | /** |
||
540 | * Do the initialization logic |
||
541 | * |
||
542 | * @return void |
||
543 | * @throws RpcExceptionInterface |
||
544 | */ |
||
545 | 2 | protected function doInitialize() |
|
563 | |||
564 | /** |
||
565 | * Marks this collection as changed/dirty. |
||
566 | * |
||
567 | * @return void |
||
568 | */ |
||
569 | 3 | private function changed() |
|
573 | } |
||
574 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.