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 DocumentCompositor 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 DocumentCompositor, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class DocumentCompositor implements |
||
22 | CompositableInterface, |
||
23 | PublishableInterface, |
||
24 | \Countable, |
||
25 | \IteratorAggregate |
||
26 | { |
||
27 | use SolidableTrait; |
||
28 | |||
29 | /** |
||
30 | * Lazy conversion from array to CompositableInterface (ie DocumentEntity). |
||
31 | * |
||
32 | * @var CompositableInterface[] |
||
33 | */ |
||
34 | private $entities = []; |
||
35 | |||
36 | /** |
||
37 | * Set of atomic operation applied to whole composition set. |
||
38 | * |
||
39 | * @var array |
||
40 | */ |
||
41 | protected $atomics = []; |
||
42 | |||
43 | /** |
||
44 | * @var string |
||
45 | */ |
||
46 | protected $class; |
||
47 | |||
48 | /** |
||
49 | * @invisible |
||
50 | * @var ODMInterface |
||
51 | */ |
||
52 | protected $odm; |
||
53 | |||
54 | /** |
||
55 | * @param string $class |
||
56 | * @param array|CompositableInterface[] $data |
||
57 | * @param ODMInterface $odm |
||
58 | */ |
||
59 | public function __construct(string $class, array $data, ODMInterface $odm) |
||
67 | |||
68 | /** |
||
69 | * Get primary composition class. |
||
70 | * |
||
71 | * @return string |
||
72 | */ |
||
73 | public function getClass(): string |
||
77 | |||
78 | /** |
||
79 | * Push new entity to end of set. |
||
80 | * |
||
81 | * Be aware that any added entity will be cloned in order to detach it from passed object: |
||
82 | * $user->addresses->push($address); |
||
83 | * $address->city = 'Minsk'; //this will have no effect of $user->addresses |
||
84 | * |
||
85 | * @param CompositableInterface $entity |
||
86 | * |
||
87 | * @return DocumentCompositor |
||
88 | * |
||
89 | * @throws CompositorException When entity is invalid type. |
||
90 | */ |
||
91 | View Code Duplication | public function push(CompositableInterface $entity): DocumentCompositor |
|
103 | |||
104 | /** |
||
105 | * Add entity to set, only one instance of document must be presented. |
||
106 | * |
||
107 | * Be aware that any added entity will be cloned in order to detach it from passed object: |
||
108 | * $user->addresses->add($address); |
||
109 | * $address->city = 'Minsk'; //this will have no effect of $user->addresses |
||
110 | * |
||
111 | * @param CompositableInterface $entity |
||
112 | * |
||
113 | * @return DocumentCompositor |
||
114 | * |
||
115 | * @throws CompositorException When entity is invalid type. |
||
116 | */ |
||
117 | View Code Duplication | public function add(CompositableInterface $entity): DocumentCompositor |
|
132 | |||
133 | /** |
||
134 | * Pull mathced entities from composition. |
||
135 | * |
||
136 | * $user->addresses->pull($address); |
||
137 | * |
||
138 | * @param CompositableInterface $entity |
||
139 | * |
||
140 | * @return DocumentCompositor |
||
141 | */ |
||
142 | View Code Duplication | public function pull(CompositableInterface $entity): DocumentCompositor |
|
155 | |||
156 | /** |
||
157 | * Check if composition contains desired document or document matching query. |
||
158 | * |
||
159 | * Example: |
||
160 | * $user->cards->has(['active' => true]); |
||
161 | * $user->cards->has(new Card(...)); |
||
162 | * |
||
163 | * @param CompositableInterface|array $query |
||
164 | * |
||
165 | * @return bool |
||
166 | */ |
||
167 | public function has($query): bool |
||
171 | |||
172 | /** |
||
173 | * Find document in composition based on given entity or matching query. |
||
174 | * |
||
175 | * $user->cards->findOne(['active' => true]); |
||
176 | * $user->cards->findOne(new Card(...)); |
||
177 | * |
||
178 | * @param CompositableInterface|array $query |
||
179 | * |
||
180 | * @return CompositableInterface|null |
||
181 | */ |
||
182 | public function findOne($query) |
||
191 | |||
192 | /** |
||
193 | * Find all entities matching given query (query can be provided in a form of |
||
194 | * CompositableInterface). |
||
195 | * |
||
196 | * $user->cards->find(['active' => true]); |
||
197 | * $user->cards->find(new Card(...)); //Attention, this will likely to return only on match |
||
198 | * |
||
199 | * @param CompositableInterface|array $query |
||
200 | * @param bool $preserveKeys Set to true to keep original offsets. |
||
201 | * |
||
202 | * @return CompositableInterface[] |
||
203 | */ |
||
204 | public function find($query, bool $preserveKeys = false): array |
||
225 | |||
226 | /** |
||
227 | * {@inheritdoc} |
||
228 | * |
||
229 | * Be aware that any added entity will be cloned in order to detach it from passed object: |
||
230 | * $user->addresses->mountValue([$address]); |
||
231 | * $address->city = 'Minsk'; //this will have no effect of $user->addresses |
||
232 | */ |
||
233 | public function stateValue($data) |
||
247 | |||
248 | /** |
||
249 | * {@inheritdoc} |
||
250 | */ |
||
251 | public function packValue(): array |
||
255 | |||
256 | /** |
||
257 | * {@inheritdoc} |
||
258 | * |
||
259 | * @param bool $changedEntities Reference, will be set to true if any of entities changed |
||
260 | * internally. |
||
261 | */ |
||
262 | public function hasUpdates(bool &$changedEntities = null): bool |
||
274 | |||
275 | /** |
||
276 | * {@inheritdoc} |
||
277 | */ |
||
278 | public function flushUpdates() |
||
286 | |||
287 | /** |
||
288 | * {@inheritdoc} |
||
289 | */ |
||
290 | public function buildAtomics(string $container = ''): array |
||
335 | |||
336 | /** |
||
337 | * Packs only public values of all nested documents. |
||
338 | * |
||
339 | * @return array |
||
340 | */ |
||
341 | public function publicFields(): array |
||
352 | |||
353 | /** |
||
354 | * {@inheritdoc} |
||
355 | */ |
||
356 | public function jsonSerialize() |
||
360 | |||
361 | /** |
||
362 | * @return int |
||
363 | */ |
||
364 | public function count(): int |
||
368 | |||
369 | /** |
||
370 | * @return \ArrayIterator |
||
371 | */ |
||
372 | public function getIterator(): \ArrayIterator |
||
376 | |||
377 | /** |
||
378 | * Cloning will be called when object will be embedded into another document. |
||
379 | */ |
||
380 | public function __clone() |
||
388 | |||
389 | /** |
||
390 | * @return array |
||
391 | */ |
||
392 | public function __debugInfo() |
||
399 | |||
400 | /** |
||
401 | * Assert that given entity supported by composition. |
||
402 | * |
||
403 | * @param mixed $entity |
||
404 | * |
||
405 | * @throws CompositorException |
||
406 | */ |
||
407 | protected function assertSupported($entity) |
||
417 | |||
418 | /** |
||
419 | * Instantiate every entity in composition. |
||
420 | * |
||
421 | * @param array $data |
||
422 | * @param bool $filter |
||
423 | * |
||
424 | * @return CompositableInterface[] |
||
425 | * |
||
426 | * @throws CompositorException |
||
427 | */ |
||
428 | private function createEntities(array $data, bool $filter = true): array |
||
444 | |||
445 | /** |
||
446 | * Pack multiple entities into array form. |
||
447 | * |
||
448 | * @param CompositableInterface[] $entities |
||
449 | * |
||
450 | * @return array |
||
451 | */ |
||
452 | private function packValues(array $entities): array |
||
461 | } |
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.