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 Stream 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 Stream, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | class Stream implements StreamInterface |
||
16 | { |
||
17 | use Type; |
||
18 | |||
19 | private $type; |
||
20 | private $spec; |
||
21 | private $values; |
||
22 | |||
23 | 228 | public function __construct(string $type) |
|
29 | |||
30 | /** |
||
31 | * {@inheritdoc} |
||
32 | */ |
||
33 | 88 | public function type(): Str |
|
37 | |||
38 | /** |
||
39 | * {@inheritdoc} |
||
40 | */ |
||
41 | 32 | public function size(): int |
|
45 | |||
46 | /** |
||
47 | * {@inheritdoc} |
||
48 | */ |
||
49 | 10 | public function count(): int |
|
53 | |||
54 | /** |
||
55 | * {@inheritdoc} |
||
56 | */ |
||
57 | 86 | public function toPrimitive() |
|
61 | |||
62 | /** |
||
63 | * {@inheritdoc} |
||
64 | */ |
||
65 | 62 | public function current() |
|
69 | |||
70 | /** |
||
71 | * {@inheritdoc} |
||
72 | */ |
||
73 | 42 | public function key() |
|
77 | |||
78 | /** |
||
79 | * {@inheritdoc} |
||
80 | */ |
||
81 | 58 | public function next() |
|
85 | |||
86 | /** |
||
87 | * {@inheritdoc} |
||
88 | */ |
||
89 | 62 | public function rewind() |
|
93 | |||
94 | /** |
||
95 | * {@inheritdoc} |
||
96 | */ |
||
97 | 62 | public function valid() |
|
101 | |||
102 | /** |
||
103 | * {@inheritdoc} |
||
104 | */ |
||
105 | 2 | public function offsetExists($offset): bool |
|
109 | |||
110 | /** |
||
111 | * {@inheritdoc} |
||
112 | */ |
||
113 | 10 | public function offsetGet($offset) |
|
117 | |||
118 | /** |
||
119 | * {@inheritdoc} |
||
120 | */ |
||
121 | 2 | public function offsetSet($offset, $value) |
|
125 | |||
126 | /** |
||
127 | * {@inheritdoc} |
||
128 | */ |
||
129 | 2 | public function offsetUnset($offset) |
|
133 | |||
134 | /** |
||
135 | * {@inheritdoc} |
||
136 | */ |
||
137 | 44 | public function get(int $index) |
|
141 | |||
142 | /** |
||
143 | * {@inheritdoc} |
||
144 | */ |
||
145 | 4 | public function diff(StreamInterface $stream): StreamInterface |
|
156 | |||
157 | /** |
||
158 | * {@inheritdoc} |
||
159 | */ |
||
160 | 2 | public function distinct(): StreamInterface |
|
167 | |||
168 | /** |
||
169 | * {@inheritdoc} |
||
170 | */ |
||
171 | 16 | public function drop(int $size): StreamInterface |
|
178 | |||
179 | /** |
||
180 | * {@inheritdoc} |
||
181 | */ |
||
182 | 2 | public function dropEnd(int $size): StreamInterface |
|
189 | |||
190 | /** |
||
191 | * {@inheritdoc} |
||
192 | */ |
||
193 | 16 | public function equals(StreamInterface $stream): bool |
|
201 | |||
202 | /** |
||
203 | * {@inheritdoc} |
||
204 | */ |
||
205 | 4 | public function filter(callable $predicate): StreamInterface |
|
212 | |||
213 | /** |
||
214 | * {@inheritdoc} |
||
215 | */ |
||
216 | public function foreach(callable $function): StreamInterface |
||
222 | |||
223 | /** |
||
224 | * {@inheritdoc} |
||
225 | */ |
||
226 | 6 | View Code Duplication | public function groupBy(callable $discriminator): MapInterface |
227 | { |
||
228 | 6 | if ($this->size() === 0) { |
|
229 | 2 | throw new GroupEmptySequenceException; |
|
230 | } |
||
231 | |||
232 | 4 | $map = null; |
|
233 | |||
234 | 4 | foreach ($this->values as $value) { |
|
235 | 4 | $key = $discriminator($value); |
|
236 | |||
237 | 4 | if ($map === null) { |
|
238 | 4 | $map = new Map( |
|
239 | 4 | $this->determineType($key), |
|
240 | 4 | StreamInterface::class |
|
241 | ); |
||
242 | } |
||
243 | |||
244 | 4 | if ($map->contains($key)) { |
|
245 | 4 | $map = $map->put( |
|
246 | $key, |
||
247 | 4 | $map->get($key)->add($value) |
|
248 | ); |
||
249 | } else { |
||
250 | 4 | $map = $map->put( |
|
251 | $key, |
||
252 | 4 | (new self((string) $this->type))->add($value) |
|
253 | ); |
||
254 | } |
||
255 | } |
||
256 | |||
257 | 4 | return $map; |
|
258 | } |
||
259 | |||
260 | /** |
||
261 | * {@inheritdoc} |
||
262 | */ |
||
263 | 4 | public function first() |
|
267 | |||
268 | /** |
||
269 | * {@inheritdoc} |
||
270 | */ |
||
271 | 4 | public function last() |
|
275 | |||
276 | /** |
||
277 | * {@inheritdoc} |
||
278 | */ |
||
279 | 98 | public function contains($element): bool |
|
283 | |||
284 | /** |
||
285 | * {@inheritdoc} |
||
286 | */ |
||
287 | 38 | public function indexOf($element): int |
|
291 | |||
292 | /** |
||
293 | * {@inheritdoc} |
||
294 | */ |
||
295 | 2 | public function indices(): StreamInterface |
|
302 | |||
303 | /** |
||
304 | * {@inheritdoc} |
||
305 | */ |
||
306 | 6 | public function map(callable $function): StreamInterface |
|
317 | |||
318 | /** |
||
319 | * {@inheritdoc} |
||
320 | */ |
||
321 | 4 | View Code Duplication | public function pad(int $size, $element): StreamInterface |
330 | |||
331 | /** |
||
332 | * {@inheritdoc} |
||
333 | */ |
||
334 | 4 | public function partition(callable $predicate): MapInterface |
|
351 | |||
352 | /** |
||
353 | * {@inheritdoc} |
||
354 | */ |
||
355 | 6 | public function slice(int $from, int $until): StreamInterface |
|
362 | |||
363 | /** |
||
364 | * {@inheritdoc} |
||
365 | */ |
||
366 | 2 | public function splitAt(int $position): StreamInterface |
|
377 | |||
378 | /** |
||
379 | * {@inheritdoc} |
||
380 | */ |
||
381 | 16 | public function take(int $size): StreamInterface |
|
388 | |||
389 | /** |
||
390 | * {@inheritdoc} |
||
391 | */ |
||
392 | 2 | public function takeEnd(int $size): StreamInterface |
|
399 | |||
400 | /** |
||
401 | * {@inheritdoc} |
||
402 | */ |
||
403 | 22 | View Code Duplication | public function append(StreamInterface $stream): StreamInterface |
414 | |||
415 | /** |
||
416 | * {@inheritdoc} |
||
417 | */ |
||
418 | 6 | View Code Duplication | public function intersect(StreamInterface $stream): StreamInterface |
429 | |||
430 | /** |
||
431 | * {@inheritdoc} |
||
432 | */ |
||
433 | 8 | public function join(string $separator): Str |
|
437 | |||
438 | /** |
||
439 | * {@inheritdoc} |
||
440 | */ |
||
441 | 176 | View Code Duplication | public function add($element): StreamInterface |
450 | |||
451 | /** |
||
452 | * {@inheritdoc} |
||
453 | */ |
||
454 | 4 | public function sort(callable $function): StreamInterface |
|
461 | |||
462 | /** |
||
463 | * {@inheritdoc} |
||
464 | */ |
||
465 | 36 | public function reduce($carry, callable $reducer) |
|
469 | |||
470 | /** |
||
471 | * {@inheritdoc} |
||
472 | */ |
||
473 | 26 | public function clear(): StreamInterface |
|
480 | |||
481 | /** |
||
482 | * {@inheritdoc} |
||
483 | */ |
||
484 | 2 | public function reverse(): StreamInterface |
|
491 | |||
492 | /** |
||
493 | * Make sure the stream is compatible with the current one |
||
494 | * |
||
495 | * @param StreamInterface $stream |
||
496 | * |
||
497 | * @throws InvalidArgumentException |
||
498 | * |
||
499 | * @return void |
||
500 | */ |
||
501 | 44 | private function validate(StreamInterface $stream) |
|
509 | } |
||
510 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: