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 RoutableSubscriber 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 RoutableSubscriber, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 39 | class RoutableSubscriber implements EventSubscriberInterface |
||
| 40 | { |
||
| 41 | const ROUTE_FIELD = 'routePath'; |
||
| 42 | const ROUTES_PROPERTY = 'suluRoutes'; |
||
| 43 | const TAG_NAME = 'sulu_article.article_route'; |
||
| 44 | |||
| 45 | /** |
||
| 46 | * @var ChainRouteGeneratorInterface |
||
| 47 | */ |
||
| 48 | private $chainRouteGenerator; |
||
| 49 | |||
| 50 | /** |
||
| 51 | * @var RouteManagerInterface |
||
| 52 | */ |
||
| 53 | private $routeManager; |
||
| 54 | |||
| 55 | /** |
||
| 56 | * @var RouteRepositoryInterface |
||
| 57 | */ |
||
| 58 | private $routeRepository; |
||
| 59 | |||
| 60 | /** |
||
| 61 | * @var EntityManagerInterface |
||
| 62 | */ |
||
| 63 | private $entityManager; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * @var DocumentManagerInterface |
||
| 67 | */ |
||
| 68 | private $documentManager; |
||
| 69 | |||
| 70 | /** |
||
| 71 | * @var DocumentInspector |
||
| 72 | */ |
||
| 73 | private $documentInspector; |
||
| 74 | |||
| 75 | /** |
||
| 76 | * @var PropertyEncoder |
||
| 77 | */ |
||
| 78 | private $propertyEncoder; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * @var StructureMetadataFactoryInterface |
||
| 82 | */ |
||
| 83 | private $metadataFactory; |
||
| 84 | |||
| 85 | /** |
||
| 86 | * @var ConflictResolverInterface |
||
| 87 | */ |
||
| 88 | private $conflictResolver; |
||
| 89 | |||
| 90 | /** |
||
| 91 | * @param ChainRouteGeneratorInterface $chainRouteGenerator |
||
| 92 | * @param RouteManagerInterface $routeManager |
||
| 93 | * @param RouteRepositoryInterface $routeRepository |
||
| 94 | * @param EntityManagerInterface $entityManager |
||
| 95 | * @param DocumentManagerInterface $documentManager |
||
| 96 | * @param DocumentInspector $documentInspector |
||
| 97 | * @param PropertyEncoder $propertyEncoder |
||
| 98 | * @param StructureMetadataFactoryInterface $metadataFactory |
||
| 99 | * @param ConflictResolverInterface $conflictResolver |
||
| 100 | */ |
||
| 101 | 52 | public function __construct( |
|
| 122 | |||
| 123 | /** |
||
| 124 | * {@inheritdoc} |
||
| 125 | */ |
||
| 126 | 45 | View Code Duplication | public static function getSubscribedEvents() |
| 142 | |||
| 143 | /** |
||
| 144 | * Load route. |
||
| 145 | * |
||
| 146 | * @param AbstractMappingEvent $event |
||
| 147 | */ |
||
| 148 | 46 | public function handleHydrate(AbstractMappingEvent $event) |
|
| 164 | |||
| 165 | /** |
||
| 166 | * Generate route and save route-path. |
||
| 167 | * |
||
| 168 | * @param AbstractMappingEvent $event |
||
| 169 | */ |
||
| 170 | 47 | public function handlePersist(AbstractMappingEvent $event) |
|
| 187 | |||
| 188 | /** |
||
| 189 | * Handle publish event and generate route and the child-routes. |
||
| 190 | * |
||
| 191 | * @param PublishEvent $event |
||
| 192 | */ |
||
| 193 | 16 | public function handlePublish(PublishEvent $event) |
|
| 227 | |||
| 228 | /** |
||
| 229 | * Create or update for given document. |
||
| 230 | * |
||
| 231 | * @param RoutablePageBehavior $document |
||
| 232 | * @param string $locale |
||
| 233 | * |
||
| 234 | * @return RouteInterface |
||
| 235 | */ |
||
| 236 | View Code Duplication | private function createOrUpdatePageRoute(RoutablePageBehavior $document, $locale) |
|
| 247 | |||
| 248 | /** |
||
| 249 | * Create or update for given document. |
||
| 250 | * |
||
| 251 | * @param RoutableBehavior $document |
||
| 252 | * @param string $locale |
||
| 253 | * |
||
| 254 | * @return RouteInterface |
||
| 255 | */ |
||
| 256 | 11 | View Code Duplication | private function createOrUpdateRoute(RoutableBehavior $document, $locale) |
| 267 | |||
| 268 | /** |
||
| 269 | * Removes old-routes where the node does not exists anymore. |
||
| 270 | * |
||
| 271 | * @param SessionInterface $session |
||
| 272 | * @param array $oldRoutes |
||
| 273 | * @param string $locale |
||
| 274 | */ |
||
| 275 | 11 | private function removeOldChildRoutes(SessionInterface $session, array $oldRoutes, $locale) |
|
| 286 | |||
| 287 | /** |
||
| 288 | * Generates child routes. |
||
| 289 | * |
||
| 290 | * @param ChildrenBehavior $document |
||
| 291 | * @param string $locale |
||
| 292 | * |
||
| 293 | * @return string[] |
||
| 294 | */ |
||
| 295 | 11 | private function generateChildRoutes(ChildrenBehavior $document, $locale) |
|
| 317 | |||
| 318 | /** |
||
| 319 | * Removes route. |
||
| 320 | * |
||
| 321 | * @param RemoveEvent $event |
||
| 322 | */ |
||
| 323 | 6 | public function handleRemove(RemoveEvent $event) |
|
| 352 | |||
| 353 | /** |
||
| 354 | * Update routes for copied article. |
||
| 355 | * |
||
| 356 | * @param CopyEvent $event |
||
| 357 | */ |
||
| 358 | 1 | public function handleCopy(CopyEvent $event) |
|
| 387 | |||
| 388 | /** |
||
| 389 | * Iterate over children and remove routes. |
||
| 390 | * |
||
| 391 | * @param ChildrenBehavior $document |
||
| 392 | */ |
||
| 393 | 1 | private function removeChildRoutes(ChildrenBehavior $document) |
|
| 405 | |||
| 406 | /** |
||
| 407 | * Removes route if exists. |
||
| 408 | * |
||
| 409 | * @param RoutablePageBehavior $document |
||
| 410 | */ |
||
| 411 | 1 | private function removeChildRoute(RoutablePageBehavior $document) |
|
| 418 | |||
| 419 | /** |
||
| 420 | * Returns encoded "routePath" property-name. |
||
| 421 | * |
||
| 422 | * @param string $structureType |
||
| 423 | * @param string $locale |
||
| 424 | * |
||
| 425 | * @return string |
||
| 426 | */ |
||
| 427 | 50 | View Code Duplication | private function getRoutePathPropertyName($structureType, $locale) |
| 437 | |||
| 438 | /** |
||
| 439 | * Returns encoded property-name. |
||
| 440 | * |
||
| 441 | * @param string $locale |
||
| 442 | * @param string $field |
||
| 443 | * |
||
| 444 | * @return string |
||
| 445 | */ |
||
| 446 | 50 | private function getPropertyName($locale, $field) |
|
| 450 | |||
| 451 | /** |
||
| 452 | * Returns true if given uuid exists. |
||
| 453 | * |
||
| 454 | * @param SessionInterface $session |
||
| 455 | * @param string $uuid |
||
| 456 | * |
||
| 457 | * @return bool |
||
| 458 | */ |
||
| 459 | private function nodeExists(SessionInterface $session, $uuid) |
||
| 469 | } |
||
| 470 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.