Complex classes like PhpcrOdmTree 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 PhpcrOdmTree, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
42 | class PhpcrOdmTree implements TreeInterface |
||
43 | { |
||
44 | /** |
||
45 | * @var ModelManager |
||
46 | */ |
||
47 | private $defaultModelManager; |
||
48 | |||
49 | /** |
||
50 | * @var DocumentManager |
||
51 | */ |
||
52 | private $dm; |
||
53 | |||
54 | /** |
||
55 | * @var Pool |
||
56 | */ |
||
57 | private $pool; |
||
58 | |||
59 | /** |
||
60 | * @var TranslatorInterface |
||
61 | */ |
||
62 | private $translator; |
||
63 | |||
64 | /** |
||
65 | * @var CoreAssetsHelper |
||
66 | */ |
||
67 | private $assetHelper; |
||
68 | |||
69 | /** |
||
70 | * Array of cached admin services indexed by class name |
||
71 | * @var array |
||
72 | */ |
||
73 | private $admins = array(); |
||
74 | |||
75 | /** |
||
76 | * List of the valid class names that may be used as tree "ref" fields |
||
77 | * @var array |
||
78 | */ |
||
79 | private $validClasses; |
||
80 | |||
81 | /** |
||
82 | * Depth to which grand children should be fetched, currently the maximum depth is one |
||
83 | * @var integer |
||
84 | */ |
||
85 | private $depth; |
||
86 | |||
87 | /** |
||
88 | * Fetch children lazy - enabling this will allow the tree to fetch a larger amount of children in the tree but less accurate |
||
89 | * @var bool |
||
90 | */ |
||
91 | private $preciseChildren; |
||
92 | |||
93 | /** |
||
94 | * The options are |
||
95 | * |
||
96 | * - depth: Down to what level children should be fetched, currently the |
||
97 | * maximum supported depth is one. |
||
98 | * - precise_children: To determine if a tree element has children, check if |
||
99 | * the document has valid children. If false, simply check if the node |
||
100 | * has any child nodes. Less accurate but better performance. |
||
101 | * |
||
102 | * @param DocumentManager $dm |
||
103 | * @param ModelManager $defaultModelManager to use with documents that have no manager |
||
104 | * @param Pool $pool to get admin classes for documents from |
||
105 | * @param TranslatorInterface $translator |
||
106 | * @param CoreAssetsHelper|AssetsHelper $assetHelper |
||
107 | * @param array $validClasses list of the valid class names that may be |
||
108 | * used as tree "ref" fields |
||
109 | * $param integer $depth depth to which grand children should be fetched, |
||
110 | * currently the maximum depth is one |
||
111 | * @param array $options |
||
112 | */ |
||
113 | public function __construct( |
||
149 | |||
150 | /** |
||
151 | * Get the children of the document at this path by looking at the Child and Children mappings. |
||
152 | * |
||
153 | * {@inheritDoc} |
||
154 | */ |
||
155 | public function getChildren($path) |
||
187 | |||
188 | /** |
||
189 | * {@inheritDoc} |
||
190 | */ |
||
191 | public function move($movedPath, $targetPath) |
||
214 | |||
215 | /** |
||
216 | * Returns an array representation of the document |
||
217 | * |
||
218 | * @param ModelManager $manager the manager to use with this document |
||
219 | * @param object $document |
||
220 | * |
||
221 | * @return array |
||
222 | */ |
||
223 | protected function documentToArray(ModelManager $manager, $document) |
||
273 | |||
274 | /** |
||
275 | * @param object $document the PHPCR-ODM document to get the sonata admin for |
||
276 | * |
||
277 | * @return AdminInterface |
||
278 | */ |
||
279 | private function getAdmin($document) |
||
280 | { |
||
281 | $className = ClassUtils::getClass($document); |
||
282 | return $this->getAdminByClass($className); |
||
283 | } |
||
284 | |||
285 | /** |
||
286 | * @param string $className |
||
287 | * |
||
288 | * @return AdminInterface |
||
289 | */ |
||
290 | private function getAdminByClass($className) |
||
291 | { |
||
292 | if (!isset($this->admins[$className])) { |
||
293 | // will return null if not defined |
||
294 | $this->admins[$className] = $this->pool->getAdminByClass($className); |
||
295 | } |
||
296 | |||
297 | return $this->admins[$className]; |
||
298 | } |
||
299 | |||
300 | /** |
||
301 | * @param ModelManager $manager the manager to use with this document |
||
302 | * @param object $document the PHPCR-ODM document to get the children of |
||
303 | * |
||
304 | * @return array of children indexed by child nodename pointing to the child documents |
||
305 | */ |
||
306 | private function getDocumentChildren(ModelManager $manager, $document) |
||
307 | { |
||
308 | $accessor = PropertyAccess::getPropertyAccessor(); // use deprecated BC method to support symfony 2.2 |
||
309 | |||
310 | /** @var $meta \Doctrine\ODM\PHPCR\Mapping\ClassMetadata */ |
||
311 | $meta = $manager->getMetadata(ClassUtils::getClass($document)); |
||
312 | |||
313 | $children = array(); |
||
314 | foreach ($meta->childrenMappings as $fieldName) { |
||
315 | try { |
||
316 | $prop = $accessor->getValue($document, $fieldName); |
||
317 | } catch (NoSuchPropertyException $e) { |
||
318 | $prop = $meta->getReflectionProperty($fieldName)->getValue($document); |
||
319 | } |
||
320 | if (null === $prop) { |
||
321 | continue; |
||
322 | } |
||
323 | if (!is_array($prop)) { |
||
324 | $prop = $prop->toArray(); |
||
325 | } |
||
326 | |||
327 | $children = array_merge($children, $this->filterDocumentChildren($document, $prop)); |
||
328 | } |
||
329 | |||
330 | foreach ($meta->childMappings as $fieldName) { |
||
331 | try { |
||
332 | $prop = $accessor->getValue($document, $fieldName); |
||
333 | } catch (NoSuchPropertyException $e) { |
||
334 | $prop = $meta->getReflectionProperty($fieldName)->getValue($document); |
||
335 | } |
||
336 | if (null !== $prop && $this->isValidDocumentChild($document, $prop)) { |
||
337 | $children[$fieldName] = $prop; |
||
338 | } |
||
339 | } |
||
340 | |||
341 | return $children; |
||
342 | } |
||
343 | |||
344 | /** |
||
345 | * @param object $document |
||
346 | * @param array $children |
||
347 | * |
||
348 | * @return array of valid children for the document |
||
349 | */ |
||
350 | protected function filterDocumentChildren($document, array $children) |
||
351 | { |
||
352 | $me = $this; |
||
353 | |||
354 | return array_filter($children, function ($child) use ($me, $document) { |
||
355 | return $me->isValidDocumentChild($document, $child); |
||
356 | }); |
||
357 | } |
||
358 | |||
359 | /** |
||
360 | * @param object $document |
||
361 | * @param object $child |
||
362 | * |
||
363 | * @return boolean TRUE if valid, FALSE if not valid |
||
364 | */ |
||
365 | public function isValidDocumentChild($document, $child) |
||
366 | { |
||
367 | $className = ClassUtils::getClass($document); |
||
368 | $childClassName = ClassUtils::getClass($child); |
||
369 | |||
370 | if (!isset($this->validClasses[$className])) { |
||
371 | // no mapping means no valid children |
||
372 | return false; |
||
373 | } |
||
374 | |||
375 | return in_array($childClassName, $this->validClasses[$className]['valid_children']); |
||
376 | } |
||
377 | |||
378 | /** |
||
379 | * {@inheritDoc} |
||
380 | */ |
||
381 | public function reorder($parent, $moved, $target, $before) |
||
387 | |||
388 | /** |
||
389 | * {@inheritDoc} |
||
390 | */ |
||
391 | public function getAlias() |
||
392 | { |
||
393 | return 'phpcr_odm_tree'; |
||
394 | } |
||
395 | |||
396 | /** |
||
397 | * {@inheritDoc} |
||
398 | */ |
||
399 | public function getNodeTypes() |
||
439 | |||
440 | /** |
||
441 | * {@inheritDoc} |
||
442 | */ |
||
443 | public function getLabels() |
||
444 | { |
||
445 | return array( |
||
446 | 'createItem' => $this->translator->trans('create_item', array(), 'SonataDoctrinePHPCRAdmin'), |
||
447 | 'deleteItem' => $this->translator->trans('delete_item', array(), 'SonataDoctrinePHPCRAdmin'), |
||
448 | ); |
||
449 | } |
||
450 | |||
451 | /** |
||
452 | * @param string $className |
||
453 | * |
||
454 | * @return string |
||
455 | */ |
||
456 | private function normalizeClassname($className) |
||
457 | { |
||
458 | return str_replace('\\', '_', $className); |
||
459 | } |
||
460 | |||
461 | /** |
||
462 | * @param string $action |
||
463 | * |
||
464 | * @return string|null |
||
465 | */ |
||
466 | private function mapAction($action) |
||
467 | { |
||
468 | switch ($action) { |
||
469 | case 'edit': return 'select_route'; |
||
470 | case 'create': return 'create_route'; |
||
471 | case 'delete': return 'delete_route'; |
||
472 | } |
||
473 | |||
474 | return null; |
||
475 | } |
||
476 | |||
477 | /** |
||
478 | * @param object $document |
||
479 | * |
||
480 | * @return ModelManager the modelmanager for $document or the default manager |
||
481 | */ |
||
482 | protected function getModelManager($document = NULL) |
||
488 | } |
||
489 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.