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 AbstractCollection 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 AbstractCollection, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | abstract class AbstractCollection implements Iterator, Countable |
||
17 | { |
||
18 | /** |
||
19 | * Models added to this collection. |
||
20 | * Tracks newly added models for rollback/change purposes. |
||
21 | * |
||
22 | * @var AbstractModel[] |
||
23 | */ |
||
24 | protected $added = []; |
||
25 | |||
26 | /** |
||
27 | * Whether the collection has been loaded with data from the persistence layer |
||
28 | * |
||
29 | * @var bool |
||
30 | */ |
||
31 | protected $loaded = true; |
||
32 | |||
33 | /** |
||
34 | * Current models assigned to this collection. |
||
35 | * Needed for iteration, access, and count purposes. |
||
36 | * |
||
37 | * @var AbstractModel[] |
||
38 | */ |
||
39 | protected $models = []; |
||
40 | |||
41 | /** |
||
42 | * Original models assigned to this collection. |
||
43 | * |
||
44 | * @var AbstractModel[] |
||
45 | */ |
||
46 | protected $original = []; |
||
47 | |||
48 | /** |
||
49 | * The array position. |
||
50 | * |
||
51 | * @var int |
||
52 | */ |
||
53 | protected $pos = 0; |
||
54 | |||
55 | /** |
||
56 | * Models removed from this collection. |
||
57 | * Tracks removed models for rollback/change purposes. |
||
58 | * |
||
59 | * @var AbstractModel[] |
||
60 | */ |
||
61 | protected $removed = []; |
||
62 | |||
63 | /** |
||
64 | * The store for handling storage operations. |
||
65 | * |
||
66 | * @var Store |
||
67 | */ |
||
68 | protected $store; |
||
69 | |||
70 | /** |
||
71 | * Constructor. |
||
72 | * |
||
73 | * @param Store $store |
||
74 | * @param AbstractModel[] $models |
||
75 | */ |
||
76 | public function __construct(Store $store, array $models = []) |
||
82 | |||
83 | /** |
||
84 | * Calculates the change set of this collection. |
||
85 | * |
||
86 | * @return array |
||
87 | */ |
||
88 | public function calculateChangeSet() |
||
98 | |||
99 | /** |
||
100 | * Clears/empties the collection. |
||
101 | * |
||
102 | * @return self |
||
103 | */ |
||
104 | public function clear() |
||
111 | |||
112 | /** |
||
113 | * {@inheritDoc} |
||
114 | */ |
||
115 | public function count() |
||
119 | |||
120 | /** |
||
121 | * {@inheritDoc} |
||
122 | */ |
||
123 | public function current() |
||
127 | |||
128 | /** |
||
129 | * Gets a single model result from the collection. |
||
130 | * |
||
131 | * @return AbstractModel|null |
||
132 | */ |
||
133 | public function getSingleResult() |
||
141 | |||
142 | /** |
||
143 | * Gets the model collection type. |
||
144 | * |
||
145 | * @return string |
||
146 | */ |
||
147 | abstract public function getType(); |
||
148 | |||
149 | /** |
||
150 | * Determines if the Model is included in the collection. |
||
151 | * |
||
152 | * @param AbstractModel $model The model to check. |
||
153 | * @return bool |
||
154 | */ |
||
155 | public function has(AbstractModel $model) |
||
159 | |||
160 | /** |
||
161 | * Determines if any models in this collection are dirty (have changes). |
||
162 | * |
||
163 | * @return bool |
||
164 | */ |
||
165 | public function hasDirtyModels() |
||
174 | |||
175 | /** |
||
176 | * Determines if the collection is dirty. |
||
177 | * |
||
178 | * @return bool |
||
179 | */ |
||
180 | public function isDirty() |
||
184 | |||
185 | /** |
||
186 | * Determines if this collection is empty. |
||
187 | * |
||
188 | * @return bool |
||
189 | */ |
||
190 | public function isEmpty() |
||
194 | |||
195 | /** |
||
196 | * Determines if models in this collection have been loaded from the persistence layer. |
||
197 | * |
||
198 | * @return bool |
||
199 | */ |
||
200 | public function isLoaded() |
||
204 | |||
205 | /** |
||
206 | * {@inheritDoc} |
||
207 | */ |
||
208 | public function key() |
||
212 | |||
213 | /** |
||
214 | * {@inheritDoc} |
||
215 | */ |
||
216 | public function next() |
||
220 | |||
221 | /** |
||
222 | * Pushes a Model into the collection. |
||
223 | * |
||
224 | * @param AbstractModel $model The model to push. |
||
225 | * @return self |
||
226 | */ |
||
227 | View Code Duplication | public function push(AbstractModel $model) |
|
245 | |||
246 | /** |
||
247 | * Removes a model from the collection. |
||
248 | * |
||
249 | * @param AbstractModel $model The model to remove. |
||
250 | * @return self |
||
251 | */ |
||
252 | View Code Duplication | public function remove(AbstractModel $model) |
|
271 | |||
272 | /** |
||
273 | * {@inheritDoc} |
||
274 | */ |
||
275 | public function rewind() |
||
279 | |||
280 | /** |
||
281 | * Rollsback the collection it it's original state. |
||
282 | * |
||
283 | * @return self |
||
284 | */ |
||
285 | public function rollback() |
||
292 | |||
293 | /** |
||
294 | * {@inheritDoc} |
||
295 | */ |
||
296 | public function valid() |
||
300 | |||
301 | /** |
||
302 | * Determines if the model is scheduled for addition to the collection. |
||
303 | * |
||
304 | * @param AbstractModel $model The model to check. |
||
305 | * @return bool |
||
306 | */ |
||
307 | public function willAdd(AbstractModel $model) |
||
311 | |||
312 | /** |
||
313 | * Determines if the model is scheduled for removal from the collection. |
||
314 | * |
||
315 | * @param AbstractModel $model The model to check. |
||
316 | * @return bool |
||
317 | */ |
||
318 | public function willRemove(AbstractModel $model) |
||
322 | |||
323 | /** |
||
324 | * Adds an model to this collection. |
||
325 | * Is used during initial collection construction. |
||
326 | * |
||
327 | * @param AbstractModel $model |
||
328 | * @return self |
||
329 | */ |
||
330 | protected function add(AbstractModel $model) |
||
345 | |||
346 | /** |
||
347 | * Evicts a model from a collection property (original, added, removed, models). |
||
348 | * |
||
349 | * @param string $property The property key |
||
350 | * @param AbstractModel $model The model to set. |
||
351 | * @return self |
||
352 | */ |
||
353 | protected function evict($property, AbstractModel $model) |
||
361 | |||
362 | /** |
||
363 | * Determines if the model is included in the original set. |
||
364 | * |
||
365 | * @param AbstractModel $model The model to check. |
||
366 | * @return bool |
||
367 | */ |
||
368 | protected function hasOriginal(AbstractModel $model) |
||
372 | |||
373 | /** |
||
374 | * Gets the Model array index from a collection property (original, added, removed, models). |
||
375 | * Will return -1 if the model was not found. |
||
376 | * |
||
377 | * @param string $property The property key |
||
378 | * @param AbstractModel $model The model to check. |
||
379 | * @return int |
||
380 | */ |
||
381 | protected function indexOf($property, AbstractModel $model) |
||
393 | |||
394 | /** |
||
395 | * Determines if the provided models match. |
||
396 | * |
||
397 | * @param AbstractModel $model |
||
398 | * @param AbstractModel $loaded |
||
399 | * @return bool |
||
400 | */ |
||
401 | abstract protected function modelsMatch(AbstractModel $model, AbstractModel $loaded); |
||
402 | |||
403 | /** |
||
404 | * Sets a model to a collection property (original, added, removed, models). |
||
405 | * |
||
406 | * @param string $property The property key |
||
407 | * @param AbstractModel $model The model to set. |
||
408 | * @return self |
||
409 | */ |
||
410 | protected function set($property, AbstractModel $model) |
||
417 | |||
418 | /** |
||
419 | * Sets an array of models to the collection. |
||
420 | * |
||
421 | * @param AbstractModel[] $models |
||
422 | * @return self |
||
423 | */ |
||
424 | protected function setModels(array $models) |
||
431 | |||
432 | /** |
||
433 | * Validates that the collection supports the incoming model. |
||
434 | * |
||
435 | * @param AbstractModel $model The model to validate. |
||
436 | * @throws \InvalidArgumentException |
||
437 | */ |
||
438 | abstract protected function validateAdd(AbstractModel $model); |
||
439 | } |
||
440 |
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.