Complex classes like Diff 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 Diff, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | class Diff extends ArrayObject implements DiffOp { |
||
24 | |||
25 | /** |
||
26 | * @var bool|null |
||
27 | */ |
||
28 | private $isAssociative = null; |
||
29 | |||
30 | /** |
||
31 | * Pointers to the operations of certain types for quick lookup. |
||
32 | * |
||
33 | * @var array[] |
||
34 | */ |
||
35 | private $typePointers = array( |
||
36 | 'add' => array(), |
||
37 | 'remove' => array(), |
||
38 | 'change' => array(), |
||
39 | 'list' => array(), |
||
40 | 'map' => array(), |
||
41 | 'diff' => array(), |
||
42 | ); |
||
43 | |||
44 | /** |
||
45 | * @var int |
||
46 | */ |
||
47 | private $indexOffset = 0; |
||
48 | |||
49 | /** |
||
50 | * @since 0.1 |
||
51 | * |
||
52 | * @param DiffOp[] $operations |
||
53 | * @param bool|null $isAssociative |
||
54 | * |
||
55 | * @throws InvalidArgumentException |
||
56 | */ |
||
57 | 105 | public function __construct( array $operations = array(), $isAssociative = null ) { |
|
74 | |||
75 | /** |
||
76 | * @since 0.1 |
||
77 | * |
||
78 | * @return DiffOp[] |
||
79 | */ |
||
80 | 122 | public function getOperations() { |
|
83 | |||
84 | /** |
||
85 | * @since 0.1 |
||
86 | * |
||
87 | * @param string $type |
||
88 | * |
||
89 | * @return DiffOp[] |
||
90 | */ |
||
91 | 22 | public function getTypeOperations( $type ) { |
|
97 | |||
98 | /** |
||
99 | * @since 0.1 |
||
100 | * |
||
101 | * @param DiffOp[] $operations |
||
102 | */ |
||
103 | 7 | public function addOperations( array $operations ) { |
|
108 | |||
109 | /** |
||
110 | * Gets called before a new element is added to the ArrayObject. |
||
111 | * |
||
112 | * At this point the index is always set (ie not null) and the |
||
113 | * value is always of the type returned by @see getObjectType. |
||
114 | * |
||
115 | * Should return a boolean. When false is returned the element |
||
116 | * does not get added to the ArrayObject. |
||
117 | * |
||
118 | * @param int|string $index |
||
119 | * @param DiffOp $value |
||
120 | * |
||
121 | * @return bool |
||
122 | * @throws InvalidArgumentException |
||
123 | */ |
||
124 | 84 | private function preSetElement( $index, DiffOp $value ) { |
|
138 | |||
139 | /** |
||
140 | * @see Serializable::unserialize |
||
141 | * |
||
142 | * @since 0.1 |
||
143 | * |
||
144 | * @param string $serialization |
||
145 | */ |
||
146 | 44 | public function unserialize( $serialization ) { |
|
147 | 44 | $serializationData = unserialize( $serialization ); |
|
148 | |||
149 | 44 | foreach ( $serializationData['data'] as $offset => $value ) { |
|
150 | // Just set the element, bypassing checks and offset resolving, |
||
151 | // as these elements have already gone through this. |
||
152 | 38 | parent::offsetSet( $offset, $value ); |
|
|
|||
153 | } |
||
154 | |||
155 | 44 | $this->indexOffset = $serializationData['index']; |
|
156 | |||
157 | 44 | $this->typePointers = $serializationData['typePointers']; |
|
158 | |||
159 | 44 | if ( array_key_exists( 'assoc', $serializationData ) ) { |
|
160 | 43 | $this->isAssociative = $serializationData['assoc'] === 'n' ? null : $serializationData['assoc'] === 't'; |
|
161 | } // The below cases are compat with < 0.4. |
||
162 | 1 | elseif ( $this instanceof MapDiff ) { |
|
163 | 1 | $this->isAssociative = true; |
|
164 | } |
||
165 | 1 | elseif ( $this instanceof ListDiff ) { |
|
166 | 1 | $this->isAssociative = false; |
|
167 | } |
||
168 | 44 | } |
|
169 | |||
170 | /** |
||
171 | * @since 0.1 |
||
172 | * |
||
173 | * @return DiffOpAdd[] |
||
174 | */ |
||
175 | 8 | public function getAdditions() { |
|
178 | |||
179 | /** |
||
180 | * @since 0.1 |
||
181 | * |
||
182 | * @return DiffOpRemove[] |
||
183 | */ |
||
184 | 8 | public function getRemovals() { |
|
187 | |||
188 | /** |
||
189 | * @since 0.1 |
||
190 | * |
||
191 | * @return DiffOpChange[] |
||
192 | */ |
||
193 | 1 | public function getChanges() { |
|
194 | 1 | return $this->getTypeOperations( 'change' ); |
|
195 | } |
||
196 | |||
197 | /** |
||
198 | * Returns the added values. |
||
199 | * |
||
200 | * @since 0.1 |
||
201 | * |
||
202 | * @return array of mixed |
||
203 | */ |
||
204 | 1 | public function getAddedValues() { |
|
212 | |||
213 | /** |
||
214 | * Returns the removed values. |
||
215 | * |
||
216 | * @since 0.1 |
||
217 | * |
||
218 | * @return array of mixed |
||
219 | */ |
||
220 | 1 | public function getRemovedValues() { |
|
228 | |||
229 | /** |
||
230 | * @see DiffOp::isAtomic |
||
231 | * |
||
232 | * @since 0.1 |
||
233 | * |
||
234 | * @return bool |
||
235 | */ |
||
236 | 72 | public function isAtomic() { |
|
239 | |||
240 | /** |
||
241 | * @see DiffOp::getType |
||
242 | * |
||
243 | * @since 0.1 |
||
244 | * |
||
245 | * @return string |
||
246 | */ |
||
247 | 152 | public function getType() { |
|
250 | |||
251 | /** |
||
252 | * Counts the number of atomic operations in the diff. |
||
253 | * This means the size of a diff with as elements only empty diffs will be 0. |
||
254 | * Or that the size of a diff with one atomic operation and one diff that itself |
||
255 | * holds two atomic operations will be 3. |
||
256 | * |
||
257 | * @see Countable::count |
||
258 | * |
||
259 | * @since 0.1 |
||
260 | * |
||
261 | * @return int |
||
262 | */ |
||
263 | 85 | public function count() { |
|
275 | |||
276 | /** |
||
277 | * @since 0.3 |
||
278 | */ |
||
279 | 1 | public function removeEmptyOperations() { |
|
286 | |||
287 | /** |
||
288 | * Returns the value of the isAssociative flag. |
||
289 | * |
||
290 | * @since 0.4 |
||
291 | * |
||
292 | * @return bool|null |
||
293 | */ |
||
294 | 10 | public function isAssociative() { |
|
297 | |||
298 | /** |
||
299 | * Returns if the diff looks associative or not. |
||
300 | * This first checks the isAssociative flag and in case its null checks |
||
301 | * if there are any non-add-non-remove operations. |
||
302 | * |
||
303 | * @since 0.4 |
||
304 | * |
||
305 | * @return bool |
||
306 | */ |
||
307 | 9 | public function looksAssociative() { |
|
310 | |||
311 | /** |
||
312 | * Returns if the diff can be non-associative. |
||
313 | * This means it does not contain any non-add-non-remove operations. |
||
314 | * |
||
315 | * @since 0.4 |
||
316 | * |
||
317 | * @return bool |
||
318 | */ |
||
319 | 16 | public function hasAssociativeOperations() { |
|
325 | |||
326 | /** |
||
327 | * Returns the Diff in array form where nested DiffOps are also turned into their array form. |
||
328 | * |
||
329 | * @see DiffOp::toArray |
||
330 | * |
||
331 | * @since 0.5 |
||
332 | * |
||
333 | * @param callable|null $valueConverter optional callback used to convert any |
||
334 | * complex values to arrays. |
||
335 | * |
||
336 | * @return array |
||
337 | */ |
||
338 | 108 | public function toArray( $valueConverter = null ) { |
|
351 | |||
352 | /** |
||
353 | * @since 2.0 |
||
354 | * |
||
355 | * @param mixed $target |
||
356 | * |
||
357 | * @return bool |
||
358 | */ |
||
359 | 14 | public function equals( $target ) { |
|
371 | |||
372 | /** |
||
373 | * Finds a new offset for when appending an element. |
||
374 | * The base class does this, so it would be better to integrate, |
||
375 | * but there does not appear to be any way to do this... |
||
376 | * |
||
377 | * @return int |
||
378 | */ |
||
379 | 23 | private function getNewOffset() { |
|
386 | |||
387 | /** |
||
388 | * @see ArrayObject::append |
||
389 | * |
||
390 | * @since 0.1 |
||
391 | * |
||
392 | * @param mixed $value |
||
393 | */ |
||
394 | 19 | public function append( $value ) { |
|
397 | |||
398 | /** |
||
399 | * @see ArrayObject::offsetSet() |
||
400 | * |
||
401 | * @since 0.1 |
||
402 | * |
||
403 | * @param int|string $index |
||
404 | * @param mixed $value |
||
405 | */ |
||
406 | 72 | public function offsetSet( $index, $value ) { |
|
409 | |||
410 | /** |
||
411 | * Method that actually sets the element and holds |
||
412 | * all common code needed for set operations, including |
||
413 | * type checking and offset resolving. |
||
414 | * |
||
415 | * If you want to do additional indexing or have code that |
||
416 | * otherwise needs to be executed whenever an element is added, |
||
417 | * you can overload @see preSetElement. |
||
418 | * |
||
419 | * @param int|string|null $index |
||
420 | * @param mixed $value |
||
421 | * |
||
422 | * @throws InvalidArgumentException |
||
423 | */ |
||
424 | 86 | private function setElement( $index, $value ) { |
|
439 | |||
440 | /** |
||
441 | * @see Serializable::serialize |
||
442 | * |
||
443 | * @since 0.1 |
||
444 | * |
||
445 | * @return string |
||
446 | */ |
||
447 | 43 | public function serialize() { |
|
459 | |||
460 | /** |
||
461 | * Returns if the ArrayObject has no elements. |
||
462 | * |
||
463 | * @since 0.1 |
||
464 | * |
||
465 | * @return bool |
||
466 | */ |
||
467 | 15 | public function isEmpty() { |
|
470 | |||
471 | } |
||
472 |
This check looks for a call to a parent method whose name is different than the method from which it is called.
Consider the following code:
The
getFirstName()
method in theSon
calls the wrong method in the parent class.