Complex classes like ByPropertyIdArray 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 ByPropertyIdArray, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
46 | class ByPropertyIdArray extends ArrayObject { |
||
47 | |||
48 | /** |
||
49 | * @var array[]|null |
||
50 | */ |
||
51 | private $byId = null; |
||
52 | |||
53 | /** |
||
54 | * @see ArrayObject::__construct |
||
55 | * |
||
56 | 65 | * @param PropertyIdProvider[]|Traversable|null $input |
|
57 | 65 | */ |
|
58 | 65 | public function __construct( $input = null ) { |
|
59 | parent::__construct( (array)$input ); |
||
60 | } |
||
61 | |||
62 | /** |
||
63 | * Builds the index for doing look-ups by property id. |
||
64 | * |
||
65 | 62 | * @since 0.2 |
|
66 | 62 | */ |
|
67 | public function buildIndex() { |
||
81 | |||
82 | /** |
||
83 | * Checks whether id indexed array has been generated. |
||
84 | 64 | * |
|
85 | 64 | * @throws RuntimeException |
|
86 | 2 | */ |
|
87 | private function assertIndexIsBuild() { |
||
88 | 62 | if ( $this->byId === null ) { |
|
89 | throw new RuntimeException( 'Index not build, call buildIndex first' ); |
||
90 | } |
||
91 | } |
||
92 | |||
93 | /** |
||
94 | * Returns the property ids used for indexing. |
||
95 | * |
||
96 | * @since 0.2 |
||
97 | * |
||
98 | 38 | * @return PropertyId[] |
|
99 | 38 | * @throws RuntimeException |
|
100 | */ |
||
101 | 37 | public function getPropertyIds() { |
|
102 | 37 | $this->assertIndexIsBuild(); |
|
103 | 37 | ||
104 | 37 | return array_map( |
|
105 | 37 | function( $serializedPropertyId ) { |
|
106 | 37 | return new PropertyId( $serializedPropertyId ); |
|
107 | }, |
||
108 | array_keys( $this->byId ) |
||
109 | ); |
||
110 | } |
||
111 | |||
112 | /** |
||
113 | * Returns the objects featuring the provided property id in the index. |
||
114 | * |
||
115 | * @since 0.2 |
||
116 | * |
||
117 | * @param PropertyId $propertyId |
||
118 | * |
||
119 | * @throws OutOfBoundsException |
||
120 | 27 | * @throws RuntimeException |
|
121 | 27 | * @return PropertyIdProvider[] |
|
122 | */ |
||
123 | 26 | public function getByPropertyId( PropertyId $propertyId ) { |
|
124 | 1 | $this->assertIndexIsBuild(); |
|
125 | |||
126 | if ( !( array_key_exists( $propertyId->getSerialization(), $this->byId ) ) ) { |
||
127 | 25 | throw new OutOfBoundsException( "Object with propertyId \"$propertyId\" not found" ); |
|
128 | } |
||
129 | |||
130 | return $this->byId[$propertyId->getSerialization()]; |
||
131 | } |
||
132 | |||
133 | /** |
||
134 | * Returns the absolute index of an object or false if the object could not be found. |
||
135 | * @since 0.5 |
||
136 | * |
||
137 | * @param PropertyIdProvider $object |
||
138 | * |
||
139 | 47 | * @return bool|int |
|
140 | 47 | * @throws RuntimeException |
|
141 | */ |
||
142 | 47 | public function getFlatArrayIndexOfObject( $object ) { |
|
143 | 47 | $this->assertIndexIsBuild(); |
|
144 | 47 | ||
145 | 47 | $i = 0; |
|
146 | foreach ( $this as $o ) { |
||
147 | 40 | if ( $o === $object ) { |
|
148 | 40 | return $i; |
|
149 | } |
||
150 | $i++; |
||
151 | } |
||
152 | return false; |
||
153 | } |
||
154 | |||
155 | /** |
||
156 | * Returns the objects in a flat array (using the indexed form for generating the array). |
||
157 | * @since 0.5 |
||
158 | * |
||
159 | * @return PropertyIdProvider[] |
||
160 | 55 | * @throws RuntimeException |
|
161 | 55 | */ |
|
162 | public function toFlatArray() { |
||
163 | 55 | $this->assertIndexIsBuild(); |
|
164 | 55 | ||
165 | 55 | $array = array(); |
|
166 | 55 | foreach ( $this->byId as $objects ) { |
|
|
|||
167 | 55 | $array = array_merge( $array, $objects ); |
|
168 | } |
||
169 | return $array; |
||
170 | } |
||
171 | |||
172 | /** |
||
173 | * Returns the absolute numeric indices of objects featuring the same property id. |
||
174 | * |
||
175 | * @param PropertyId $propertyId |
||
176 | * |
||
177 | * @throws RuntimeException |
||
178 | 43 | * @return int[] |
|
179 | 43 | */ |
|
180 | private function getFlatArrayIndices( PropertyId $propertyId ) { |
||
181 | 43 | $this->assertIndexIsBuild(); |
|
182 | 43 | ||
183 | $propertyIndices = array(); |
||
184 | 43 | $i = 0; |
|
185 | 41 | ||
186 | 41 | foreach ( $this->byId as $serializedPropertyId => $objects ) { |
|
187 | 41 | if ( $serializedPropertyId === $propertyId->getSerialization() ) { |
|
188 | $propertyIndices = range( $i, $i + count( $objects ) - 1 ); |
||
189 | 29 | break; |
|
190 | } else { |
||
191 | 43 | $i += count( $objects ); |
|
192 | } |
||
193 | 43 | } |
|
194 | |||
195 | return $propertyIndices; |
||
196 | } |
||
197 | |||
198 | /** |
||
199 | * Moves an object within its "property group". |
||
200 | * |
||
201 | * @param PropertyIdProvider $object |
||
202 | * @param int $toIndex Absolute index within a "property group". |
||
203 | * |
||
204 | 38 | * @throws OutOfBoundsException |
|
205 | 38 | */ |
|
206 | private function moveObjectInPropertyGroup( $object, $toIndex ) { |
||
207 | 38 | $currentIndex = $this->getFlatArrayIndexOfObject( $object ); |
|
208 | 17 | ||
209 | if ( $toIndex === $currentIndex ) { |
||
210 | return; |
||
211 | 21 | } |
|
212 | |||
213 | 21 | $propertyId = $object->getPropertyId(); |
|
214 | 21 | ||
215 | $numericIndices = $this->getFlatArrayIndices( $propertyId ); |
||
216 | 21 | $lastIndex = $numericIndices[count( $numericIndices ) - 1]; |
|
217 | |||
218 | if ( $toIndex > $lastIndex + 1 || $toIndex < $numericIndices[0] ) { |
||
219 | throw new OutOfBoundsException( 'Object cannot be moved to ' . $toIndex ); |
||
220 | 21 | } |
|
221 | 12 | ||
222 | 12 | if ( $toIndex >= $lastIndex ) { |
|
223 | 9 | $this->moveObjectToEndOfPropertyGroup( $object ); |
|
224 | } else { |
||
225 | 9 | $this->removeObject( $object ); |
|
226 | 9 | ||
227 | 9 | $propertyGroup = array_combine( |
|
228 | 9 | $this->getFlatArrayIndices( $propertyId ), |
|
229 | $this->getByPropertyId( $propertyId ) |
||
230 | 9 | ); |
|
231 | 9 | ||
232 | $insertBefore = $propertyGroup[$toIndex]; |
||
233 | 21 | $this->insertObjectAtIndex( $object, $this->getFlatArrayIndexOfObject( $insertBefore ) ); |
|
234 | } |
||
235 | } |
||
236 | |||
237 | /** |
||
238 | * Moves an object to the end of its "property group". |
||
239 | * |
||
240 | 12 | * @param PropertyIdProvider $object |
|
241 | 12 | */ |
|
242 | private function moveObjectToEndOfPropertyGroup( $object ) { |
||
257 | |||
258 | /** |
||
259 | * Removes an object from the array structures. |
||
260 | * |
||
261 | * @param PropertyIdProvider $object |
||
262 | 23 | */ |
|
263 | 23 | private function removeObject( $object ) { |
|
264 | 23 | $flatArray = $this->toFlatArray(); |
|
265 | 23 | $this->exchangeArray( $flatArray ); |
|
266 | 23 | $this->offsetUnset( array_search( $object, $flatArray ) ); |
|
267 | 23 | $this->buildIndex(); |
|
268 | } |
||
269 | |||
270 | /** |
||
271 | * Inserts an object at a specific index. |
||
272 | * |
||
273 | * @param PropertyIdProvider $object |
||
274 | * @param int $index Absolute index within the flat list of objects. |
||
275 | 9 | */ |
|
276 | 9 | private function insertObjectAtIndex( $object, $index ) { |
|
277 | $flatArray = $this->toFlatArray(); |
||
278 | 9 | ||
279 | 9 | $this->exchangeArray( array_merge( |
|
280 | 9 | array_slice( $flatArray, 0, $index ), |
|
281 | 9 | array( $object ), |
|
282 | 9 | array_slice( $flatArray, $index ) |
|
283 | ) ); |
||
284 | 9 | ||
285 | 9 | $this->buildIndex(); |
|
286 | } |
||
287 | |||
288 | /** |
||
289 | * @param PropertyId $propertyId |
||
290 | * @param int $toIndex |
||
291 | 32 | */ |
|
292 | 32 | private function movePropertyGroup( PropertyId $propertyId, $toIndex ) { |
|
293 | if ( $this->getPropertyGroupIndex( $propertyId ) === $toIndex ) { |
||
294 | return; |
||
295 | } |
||
296 | 32 | ||
297 | $insertBefore = null; |
||
298 | 32 | ||
299 | 32 | $oldIndex = $this->getPropertyGroupIndex( $propertyId ); |
|
300 | $byIdClone = $this->byId; |
||
301 | |||
302 | 32 | // Remove "property group" to calculate the groups new index: |
|
303 | unset( $this->byId[$propertyId->getSerialization()] ); |
||
304 | 32 | ||
305 | if ( $toIndex > $oldIndex ) { |
||
306 | // If the group shall be moved towards the bottom, the number of objects within the |
||
307 | 17 | // group needs to be subtracted from the absolute toIndex: |
|
308 | 17 | $toIndex -= count( $byIdClone[$propertyId->getSerialization()] ); |
|
309 | } |
||
310 | 32 | ||
311 | foreach ( $this->getPropertyIds() as $pId ) { |
||
312 | // Accepting other than the exact index by using <= letting the "property group" "latch" |
||
313 | 32 | // in the next slot. |
|
314 | 24 | if ( $toIndex <= $this->getPropertyGroupIndex( $pId ) ) { |
|
315 | 24 | $insertBefore = $pId; |
|
316 | break; |
||
317 | 32 | } |
|
318 | } |
||
319 | 32 | ||
320 | 32 | $serializedPropertyId = $propertyId->getSerialization(); |
|
321 | $this->byId = array(); |
||
322 | 32 | ||
323 | 32 | foreach ( $byIdClone as $serializedPId => $objects ) { |
|
324 | 32 | $pId = new PropertyId( $serializedPId ); |
|
325 | 32 | if ( $pId->equals( $propertyId ) ) { |
|
326 | 32 | continue; |
|
327 | 24 | } elseif ( $pId->equals( $insertBefore ) ) { |
|
328 | 24 | $this->byId[$serializedPropertyId] = $byIdClone[$serializedPropertyId]; |
|
329 | 32 | } |
|
330 | 32 | $this->byId[$serializedPId] = $objects; |
|
331 | } |
||
332 | 32 | ||
333 | 8 | if ( $insertBefore === null ) { |
|
334 | 8 | $this->byId[$serializedPropertyId] = $byIdClone[$serializedPropertyId]; |
|
335 | } |
||
336 | 32 | ||
337 | 32 | $this->exchangeArray( $this->toFlatArray() ); |
|
338 | } |
||
339 | |||
340 | /** |
||
341 | * Returns the index of a "property group" (the first object in the flat array that features |
||
342 | * the specified property). Returns false if property id could not be found. |
||
343 | * |
||
344 | * @param PropertyId $propertyId |
||
345 | * |
||
346 | * @return bool|int |
||
347 | 32 | */ |
|
348 | 32 | private function getPropertyGroupIndex( PropertyId $propertyId ) { |
|
349 | $i = 0; |
||
350 | 32 | ||
351 | 32 | foreach ( $this->byId as $serializedPropertyId => $objects ) { |
|
352 | 32 | $pId = new PropertyId( $serializedPropertyId ); |
|
353 | 32 | if ( $pId->equals( $propertyId ) ) { |
|
354 | return $i; |
||
355 | 30 | } |
|
356 | 30 | $i += count( $objects ); |
|
357 | } |
||
358 | |||
359 | return false; |
||
360 | } |
||
361 | |||
362 | /** |
||
363 | * Moves an existing object to a new index. Specifying an index outside the object's "property |
||
364 | * group" will move the object to the edge of the "property group" and shift the whole group |
||
365 | * to achieve the designated index for the object to move. |
||
366 | * @since 0.5 |
||
367 | * |
||
368 | * @param PropertyIdProvider $object |
||
369 | * @param int $toIndex Absolute index where to move the object to. |
||
370 | * |
||
371 | * @throws OutOfBoundsException |
||
372 | * @throws RuntimeException |
||
373 | 47 | */ |
|
374 | 47 | public function moveObjectToIndex( $object, $toIndex ) { |
|
375 | $this->assertIndexIsBuild(); |
||
376 | 47 | ||
377 | 1 | if ( !in_array( $object, $this->toFlatArray() ) ) { |
|
378 | 46 | throw new OutOfBoundsException( 'Object not present in array' ); |
|
379 | 1 | } elseif ( $toIndex < 0 || $toIndex > count( $this ) ) { |
|
380 | 45 | throw new OutOfBoundsException( 'Specified index is out of bounds' ); |
|
381 | 7 | } elseif ( $this->getFlatArrayIndexOfObject( $object ) === $toIndex ) { |
|
382 | return; |
||
383 | } |
||
384 | |||
385 | 38 | // Determine whether to simply reindex the object within its "property group": |
|
386 | $propertyIndices = $this->getFlatArrayIndices( $object->getPropertyId() ); |
||
387 | 38 | ||
388 | 8 | if ( in_array( $toIndex, $propertyIndices ) ) { |
|
389 | 8 | $this->moveObjectInPropertyGroup( $object, $toIndex ); |
|
390 | 30 | } else { |
|
391 | 30 | $edgeIndex = ( $toIndex <= $propertyIndices[0] ) |
|
392 | 30 | ? $propertyIndices[0] |
|
393 | : $propertyIndices[count( $propertyIndices ) - 1]; |
||
394 | 30 | ||
395 | 30 | $this->moveObjectInPropertyGroup( $object, $edgeIndex ); |
|
396 | $this->movePropertyGroup( $object->getPropertyId(), $toIndex ); |
||
397 | } |
||
398 | 38 | ||
399 | 38 | $this->exchangeArray( $this->toFlatArray() ); |
|
400 | } |
||
401 | |||
402 | /** |
||
403 | * Adds an object at a specific index. If no index is specified, the object will be append to |
||
404 | * the end of its "property group" or - if no objects featuring the same property exist - to the |
||
405 | * absolute end of the array. |
||
406 | * Specifying an index outside a "property group" will place the new object at the specified |
||
407 | * index with the existing "property group" objects being shifted towards the new object. |
||
408 | * |
||
409 | * @since 0.5 |
||
410 | * |
||
411 | * @param PropertyIdProvider $object |
||
412 | * @param int|null $index Absolute index where to place the new object. |
||
413 | * |
||
414 | 7 | * @throws RuntimeException |
|
415 | 7 | */ |
|
416 | public function addObjectAtIndex( $object, $index = null ) { |
||
417 | 7 | $this->assertIndexIsBuild(); |
|
418 | 7 | ||
419 | $propertyId = $object->getPropertyId(); |
||
420 | 7 | $validIndices = $this->getFlatArrayIndices( $propertyId ); |
|
421 | |||
422 | 2 | if ( count( $this ) === 0 ) { |
|
423 | 7 | // Array is empty, just append object. |
|
424 | $this->append( $object ); |
||
425 | } elseif ( empty( $validIndices ) ) { |
||
426 | 2 | // No objects featuring that property exist. The object may be inserted at a place |
|
427 | 2 | // between existing "property groups". |
|
428 | 2 | $this->append( $object ); |
|
429 | 2 | if ( $index !== null ) { |
|
430 | 2 | $this->buildIndex(); |
|
431 | 2 | $this->moveObjectToIndex( $object, $index ); |
|
432 | } |
||
433 | } else { |
||
434 | 3 | // Objects featuring the same property as the object which is about to be added already |
|
435 | // exist in the array. |
||
436 | $this->addObjectToPropertyGroup( $object, $index ); |
||
437 | 7 | } |
|
438 | 7 | ||
439 | $this->buildIndex(); |
||
440 | } |
||
441 | |||
442 | /** |
||
443 | * Adds an object to an existing property group at the specified absolute index. |
||
444 | * |
||
445 | * @param PropertyIdProvider $object |
||
446 | * @param int|null $index |
||
447 | * |
||
448 | 3 | * @throws OutOfBoundsException |
|
449 | */ |
||
450 | 3 | private function addObjectToPropertyGroup( $object, $index = null ) { |
|
487 | |||
488 | } |
||
489 |
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.