Complex classes like Collection 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 Collection, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
22 | class Collection implements \Iterator, \ArrayAccess, \Countable |
||
23 | { |
||
24 | /** |
||
25 | * @var array |
||
26 | */ |
||
27 | protected $list = []; |
||
28 | |||
29 | /** |
||
30 | * @var int |
||
31 | */ |
||
32 | protected $position = 0; |
||
33 | |||
34 | /** |
||
35 | * this is required for the unset() <=> offsetUnset() where an exception is |
||
36 | * thrown if next() or rewind() is not called after unset. |
||
37 | * @var bool |
||
38 | */ |
||
39 | protected $next_or_rewind_required = false; |
||
40 | |||
41 | 87 | public function __construct() |
|
45 | |||
46 | /** |
||
47 | * @return int |
||
48 | */ |
||
49 | 12 | public function count() |
|
53 | |||
54 | /** |
||
55 | * This method is meant to be overridden by child Classes in order to throw |
||
56 | * an Exception if the value can't be added, else it will be added normally. |
||
57 | * |
||
58 | * @param mixed $value |
||
59 | */ |
||
60 | 66 | protected function preAdd($value) |
|
63 | |||
64 | /** |
||
65 | * Get the next numeric offset. |
||
66 | * |
||
67 | * we get the MAX numeric offset we have and increment it by 1. |
||
68 | */ |
||
69 | 57 | protected function nextOffset() |
|
80 | |||
81 | /** |
||
82 | * @param string|int $offset |
||
83 | * @param mixed $value |
||
84 | * |
||
85 | * @throws \Exception |
||
86 | */ |
||
87 | 72 | public function offsetSet($offset, $value) |
|
96 | |||
97 | /** |
||
98 | * This is not in the interface, add an item. |
||
99 | * @return self |
||
100 | * @throws \Exception |
||
101 | */ |
||
102 | 12 | public function add($item) |
|
107 | |||
108 | /** |
||
109 | * This is not in the interface, add an item passing it by reference. |
||
110 | * |
||
111 | * This will only accept variables as input. |
||
112 | * |
||
113 | * @return self |
||
114 | * @throws \Exception |
||
115 | */ |
||
116 | 3 | public function addByReference(&$item) |
|
122 | |||
123 | /** |
||
124 | * @param string|int $offset |
||
125 | * |
||
126 | * @return mixed|null |
||
127 | */ |
||
128 | 21 | public function &offsetGet($offset) |
|
129 | { |
||
130 | 21 | if (array_key_exists($offset, $this->list)) { |
|
131 | 18 | return $this->list[$offset]; |
|
132 | } |
||
133 | 3 | $null = null; |
|
134 | 3 | return $null; |
|
135 | } |
||
136 | |||
137 | /** |
||
138 | * @return bool |
||
139 | */ |
||
140 | 6 | public function offsetExists($offset) |
|
144 | |||
145 | 12 | public function offsetUnset($offset) |
|
156 | |||
157 | 18 | public function rewind() |
|
162 | |||
163 | 9 | public function next() |
|
168 | |||
169 | /** |
||
170 | * This is not in the interface, go back in the array. |
||
171 | */ |
||
172 | 3 | public function prev() |
|
173 | { |
||
174 | 3 | --$this->position; |
|
175 | 3 | $this->next_or_rewind_required = false; |
|
176 | 3 | } |
|
177 | |||
178 | /** |
||
179 | * @return bool |
||
180 | */ |
||
181 | 12 | public function valid() |
|
182 | { |
||
183 | 12 | if (empty($this->list)) { |
|
184 | 6 | return false; |
|
185 | } |
||
186 | 6 | return array_key_exists($this->position, array_keys($this->list)); |
|
187 | } |
||
188 | |||
189 | 21 | public function current() |
|
190 | { |
||
191 | 21 | if ($this->next_or_rewind_required) { |
|
192 | 3 | throw new \Exception('Calling next or rewind is required after call to unset, current item can\'t be accessed otherwise because it is removed.'); |
|
193 | } |
||
194 | 18 | return $this->list[$this->key()]; |
|
195 | } |
||
196 | |||
197 | /** |
||
198 | * @return string |
||
199 | */ |
||
200 | 18 | public function key() |
|
206 | |||
207 | /** |
||
208 | * @return int |
||
209 | */ |
||
210 | 3 | public function position() |
|
214 | |||
215 | /** |
||
216 | * This is not in the interface, Get the last item. |
||
217 | */ |
||
218 | 6 | public function last() |
|
219 | { |
||
220 | 6 | $keys = array_keys($this->list); |
|
221 | 6 | $offset = array_pop($keys); |
|
222 | 6 | if (array_key_exists($offset, $this->list)) { |
|
223 | 3 | return $this->list[$offset]; |
|
224 | } |
||
225 | 3 | $null = null; |
|
226 | 3 | return $null; |
|
227 | } |
||
228 | |||
229 | /** |
||
230 | * This is not in the interface, Get the first item. |
||
231 | */ |
||
232 | 6 | public function first() |
|
233 | { |
||
234 | 6 | $keys = array_keys($this->list); |
|
235 | 6 | $offset = array_shift($keys); |
|
236 | 6 | if (array_key_exists($offset, $this->list)) { |
|
237 | 3 | return $this->list[$offset]; |
|
238 | } |
||
239 | 3 | $null = null; |
|
240 | 3 | return $null; |
|
241 | } |
||
242 | |||
243 | /** |
||
244 | * This is not in the interface, Return the keys of the internal list. |
||
245 | * |
||
246 | * @return array |
||
247 | */ |
||
248 | 9 | public function keys() |
|
249 | { |
||
250 | 9 | return array_keys($this->list); |
|
251 | } |
||
252 | |||
253 | /** |
||
254 | * This is not in the interface, cast the internal list to an array. |
||
255 | * |
||
256 | * @return array |
||
257 | */ |
||
258 | 24 | public function toArray() |
|
262 | |||
263 | |||
264 | /** |
||
265 | * This is not in the interface, merge this object with another object and act |
||
266 | * like array_merge, numeric indexes are added, other indexes are overridden. |
||
267 | * |
||
268 | * This method will alter the current object. |
||
269 | * |
||
270 | * @param Collection $another_list |
||
271 | * |
||
272 | * @return self |
||
273 | * |
||
274 | * @throws \Exception |
||
275 | * |
||
276 | */ |
||
277 | 3 | public function merge(Collection $another_list) |
|
278 | { |
||
279 | 3 | for ($another_list->rewind(); $another_list->valid(); $another_list->next()) { |
|
280 | $key = $another_list->key(); |
||
281 | try { |
||
282 | $this->offsetSet(is_numeric($key) ? null : $key, $another_list->current()); |
||
283 | } catch (\Exception $e) { |
||
284 | throw $e; |
||
285 | } |
||
286 | } |
||
287 | 3 | return $this; |
|
288 | } |
||
289 | |||
290 | /** |
||
291 | * Change the position of item at $offset1 in the internal array to be at |
||
292 | * the position of item at $offset2 and vise versa. |
||
293 | * |
||
294 | * @param $offset1 |
||
295 | * @param $offset2 |
||
296 | * |
||
297 | * @return $this |
||
298 | * @throws \Exception |
||
299 | */ |
||
300 | 9 | public function swap($offset1, $offset2) |
|
301 | { |
||
302 | 9 | $pos1 = $this->offsetPosition($offset1); |
|
303 | 9 | $pos2 = $this->offsetPosition($offset2); |
|
304 | 9 | if ($pos1 === false || $pos2 === false) { |
|
305 | // offset does not exist |
||
306 | 3 | throw new \Exception("Trying to swap non exist offsets"); |
|
307 | } |
||
308 | 6 | if ($pos1 == $pos2) { |
|
309 | 3 | return $this; |
|
310 | } |
||
311 | 3 | $temp = $this[$offset1]; |
|
312 | 3 | $this[$offset1] = $this[$offset2]; |
|
313 | 3 | $this[$offset2] = $temp; |
|
314 | |||
315 | 3 | return $this; |
|
316 | } |
||
317 | |||
318 | /** |
||
319 | * Change the position of item at $offset1 in the internal array to be at |
||
320 | * the position of item at $offset2 and vise versa using re-order and refilling |
||
321 | * the array |
||
322 | * |
||
323 | * @param $offset1 |
||
324 | * @param $offset2 |
||
325 | * |
||
326 | * @return self |
||
327 | * |
||
328 | * @throws \Exception |
||
329 | */ |
||
330 | 9 | public function swapReorder($offset1, $offset2) |
|
331 | { |
||
332 | 9 | $pos1 = $this->offsetPosition($offset1); |
|
333 | 9 | $pos2 = $this->offsetPosition($offset2); |
|
334 | 9 | if ($pos1 === false || $pos2 === false) { |
|
335 | // offset does not exist |
||
336 | 3 | throw new \Exception("Trying to swap non exist offsets"); |
|
337 | } |
||
338 | 6 | if ($pos1 == $pos2) { |
|
339 | 3 | return $this; |
|
340 | } |
||
341 | |||
342 | 3 | $clone = clone $this; |
|
343 | 3 | for ($clone->rewind(); $clone->valid(); $clone->next()) { |
|
344 | 3 | $key = $clone->key(); |
|
345 | 3 | $curr_pos = $clone->position(); |
|
346 | 3 | if ($curr_pos === $pos1) { |
|
347 | 3 | $this[$key] = $clone->offsetGet($offset2); |
|
348 | 3 | } elseif ($curr_pos === $pos2) { |
|
349 | 3 | $this[$key] = $clone->offsetGet($offset1); |
|
350 | 1 | } else { |
|
351 | 3 | $this[$key] = $clone->current(); |
|
352 | } |
||
353 | 1 | } |
|
354 | |||
355 | 3 | return $this; |
|
356 | } |
||
357 | |||
358 | |||
359 | /** |
||
360 | * Get the item by internal pointer numeric position and not by the offset. |
||
361 | * |
||
362 | * This is not in the interface. |
||
363 | * |
||
364 | * @param int $pos |
||
365 | * |
||
366 | * @return null|mixed |
||
367 | */ |
||
368 | 6 | public function byPositionGet($pos) |
|
369 | { |
||
370 | 6 | $keys = $this->keys(); |
|
371 | 6 | if (array_key_exists($pos, $keys)) { |
|
372 | 3 | $offset = $keys[$pos]; |
|
373 | 3 | return $this->list[$offset]; |
|
374 | } |
||
375 | |||
376 | 3 | return null; |
|
377 | } |
||
378 | |||
379 | /** |
||
380 | * Get the internal pointer numeric position which corresponds to the |
||
381 | * offset passed. |
||
382 | * this is not in the interface |
||
383 | * |
||
384 | * @param $offset |
||
385 | * |
||
386 | * @return int|FALSE |
||
387 | */ |
||
388 | 27 | public function offsetPosition($offset) |
|
394 | |||
395 | /** |
||
396 | * Check if this container is empty. |
||
397 | * |
||
398 | * This is not in the interface |
||
399 | * @return bool |
||
400 | */ |
||
401 | 6 | public function isEmpty() |
|
402 | { |
||
403 | 6 | return count($this) === 0; |
|
404 | } |
||
405 | |||
406 | /** |
||
407 | * Perform array_diff_key against another list where the first parameter is |
||
408 | * this collection and the second is the new list. |
||
409 | * |
||
410 | * This will return a new Collection. |
||
411 | * |
||
412 | * @param Collection $another_list |
||
413 | * |
||
414 | * @return Collection |
||
415 | */ |
||
416 | 3 | public function diffKey(Collection $another_list) |
|
425 | |||
426 | /** |
||
427 | * A version of php array_diff_assoc which is recursive. |
||
428 | * |
||
429 | * array_diff_assoc — Computes the difference of arrays with additional index check |
||
430 | * |
||
431 | * @param array $array1 |
||
432 | * @param array $array2 |
||
433 | * |
||
434 | * @return array |
||
435 | */ |
||
436 | 9 | protected static function arrayDiffRecursiveAssoc(array $array1, array $array2) |
|
437 | { |
||
438 | 9 | $ret = array(); |
|
439 | |||
440 | 9 | foreach ($array1 as $key1 => $value1) { |
|
441 | 9 | if (!array_key_exists($key1, $array2)) { |
|
442 | 6 | $ret[$key1] = $value1; |
|
443 | 6 | continue; |
|
444 | } |
||
445 | 3 | if (is_array($value1) && is_array($array2[$key1])) { |
|
446 | $recursive_diff = self::arrayDiffRecursiveAssoc($value1, $array2[$key1]); |
||
447 | if (count($recursive_diff)) { |
||
448 | $ret[$key1] = $recursive_diff; |
||
449 | } |
||
450 | 3 | } elseif ($value1 instanceof Collection && $array2[$key1] instanceof Collection) { |
|
451 | 3 | $recursive_diff = $value1->diffRecursiveAssoc($array2[$key1]); |
|
452 | 3 | if ($recursive_diff->count()) { |
|
453 | 2 | $ret[$key1] = $recursive_diff; |
|
454 | } |
||
455 | 3 | } elseif (!is_array($value1) && !is_array($array2[$key1])) { |
|
456 | 3 | if ($value1 !== $array2[$key1]) { |
|
457 | 2 | $ret[$key1] = $value1; |
|
458 | } |
||
459 | 1 | } else { |
|
460 | 1 | $ret[$key1] = $value1; |
|
461 | } |
||
462 | 3 | } |
|
463 | 9 | return $ret; |
|
464 | } |
||
465 | |||
466 | /** |
||
467 | * Return Recursive diff from another list with index check. |
||
468 | * |
||
469 | * The first array is this collection and compute the diff against the passed collection. |
||
470 | * |
||
471 | * This will return a new Collection. |
||
472 | * |
||
473 | * @param Collection $another_list |
||
474 | * |
||
475 | * @return static |
||
476 | */ |
||
477 | 9 | public function diffRecursiveAssoc(Collection $another_list) |
|
478 | { |
||
479 | 9 | $ret = new static(); |
|
480 | 9 | $diff = static::arrayDiffRecursiveAssoc($this->toArray(), $another_list->toArray()); |
|
481 | 9 | foreach ($diff as $key => $value) { |
|
482 | 6 | $ret[$key] = $value; |
|
483 | 3 | } |
|
484 | 9 | return $ret; |
|
485 | } |
||
486 | |||
487 | /** |
||
488 | * A version of php array_diff which is recursive. |
||
489 | * |
||
490 | * array_diff — Computes the difference of arrays |
||
491 | * |
||
492 | * This function is costy because it uses array_search |
||
493 | * |
||
494 | * @param array $array1 |
||
495 | * @param array $array2 |
||
496 | * |
||
497 | * @return array |
||
498 | */ |
||
499 | 3 | protected static function arrayDiffRecursive(array $array1, array $array2) |
|
528 | |||
529 | /** |
||
530 | * Return Recursive diff from another list. |
||
531 | * |
||
532 | * The first array is this collection and compute the diff against the passed collection. |
||
533 | * |
||
534 | * This will return a new Collection. |
||
535 | * |
||
536 | * @param Collection $another_list |
||
537 | * |
||
538 | * @return static |
||
539 | */ |
||
540 | 3 | public function diffRecursive(Collection $another_list) |
|
549 | |||
550 | /** |
||
551 | * Implementation of array_intesect_key against another list. |
||
552 | * |
||
553 | * This will return a new Collection. |
||
554 | * |
||
555 | * @param Collection $another_list |
||
556 | * |
||
557 | * @return static |
||
558 | */ |
||
559 | 3 | public function intersectKey(Collection $another_list) |
|
569 | } |
||
570 |
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.