ezsystems /
ezpublish-kernel
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 ExceptionConversion 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 ExceptionConversion, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 23 | class ExceptionConversion extends Gateway |
||
| 24 | { |
||
| 25 | /** |
||
| 26 | * The wrapped gateway. |
||
| 27 | * |
||
| 28 | * @var Gateway |
||
| 29 | */ |
||
| 30 | protected $innerGateway; |
||
| 31 | |||
| 32 | /** |
||
| 33 | * Creates a new exception conversion gateway around $innerGateway. |
||
| 34 | * |
||
| 35 | * @param Gateway $innerGateway |
||
| 36 | */ |
||
| 37 | public function __construct(Gateway $innerGateway) |
||
| 41 | |||
| 42 | /** |
||
| 43 | * Returns an array with basic node data. |
||
| 44 | * |
||
| 45 | * We might want to cache this, since this method is used by about every |
||
| 46 | * method in the location handler. |
||
| 47 | * |
||
| 48 | * @todo optimize |
||
| 49 | * |
||
| 50 | * @param mixed $nodeId |
||
| 51 | * |
||
| 52 | * @return array |
||
| 53 | */ |
||
| 54 | public function getBasicNodeData($nodeId) |
||
| 64 | |||
| 65 | /** |
||
| 66 | * Returns an array with basic node data for the node with $remoteId. |
||
| 67 | * |
||
| 68 | * @todo optimize |
||
| 69 | * |
||
| 70 | * @param mixed $remoteId |
||
| 71 | * |
||
| 72 | * @return array |
||
| 73 | */ |
||
| 74 | public function getBasicNodeDataByRemoteId($remoteId) |
||
| 84 | |||
| 85 | /** |
||
| 86 | * Returns total count and data for all Locations satisfying the parameters. |
||
| 87 | * |
||
| 88 | * @param \eZ\Publish\API\Repository\Values\Content\Query\Criterion $criterion |
||
| 89 | * @param int $offset |
||
| 90 | * @param int|null $limit |
||
| 91 | * @param \eZ\Publish\API\Repository\Values\Content\Query\SortClause[] $sortClauses |
||
| 92 | * |
||
| 93 | * @return mixed[][] |
||
| 94 | */ |
||
| 95 | View Code Duplication | public function find(Criterion $criterion, $offset = 0, $limit = null, array $sortClauses = null) |
|
| 96 | { |
||
| 97 | try { |
||
| 98 | return $this->innerGateway->find($criterion, $offset, $limit, $sortClauses); |
||
|
|
|||
| 99 | } catch (DBALException $e) { |
||
| 100 | throw new RuntimeException('Database error', 0, $e); |
||
| 101 | } catch (PDOException $e) { |
||
| 102 | throw new RuntimeException('Database error', 0, $e); |
||
| 103 | } |
||
| 104 | } |
||
| 105 | |||
| 106 | /** |
||
| 107 | * Loads data for all Locations for $contentId, optionally only in the |
||
| 108 | * subtree starting at $rootLocationId. |
||
| 109 | * |
||
| 110 | * @param int $contentId |
||
| 111 | * @param int $rootLocationId |
||
| 112 | * |
||
| 113 | * @return array |
||
| 114 | */ |
||
| 115 | public function loadLocationDataByContent($contentId, $rootLocationId = null) |
||
| 125 | |||
| 126 | /** |
||
| 127 | * @see \eZ\Publish\Core\Persistence\Legacy\Content\Location\Gateway::loadParentLocationsDataForDraftContent |
||
| 128 | */ |
||
| 129 | public function loadParentLocationsDataForDraftContent($contentId) |
||
| 139 | |||
| 140 | /** |
||
| 141 | * Find all content in the given subtree. |
||
| 142 | * |
||
| 143 | * @param mixed $sourceId |
||
| 144 | * @param bool $onlyIds |
||
| 145 | * |
||
| 146 | * @return array |
||
| 147 | */ |
||
| 148 | public function getSubtreeContent($sourceId, $onlyIds = false) |
||
| 158 | |||
| 159 | /** |
||
| 160 | * Returns data for the first level children of the location identified by given $locationId. |
||
| 161 | * |
||
| 162 | * @param mixed $locationId |
||
| 163 | * |
||
| 164 | * @return array |
||
| 165 | */ |
||
| 166 | public function getChildren($locationId) |
||
| 176 | |||
| 177 | /** |
||
| 178 | * Update path strings to move nodes in the ezcontentobject_tree table. |
||
| 179 | * |
||
| 180 | * This query can likely be optimized to use some more advanced string |
||
| 181 | * operations, which then depend on the respective database. |
||
| 182 | * |
||
| 183 | * @todo optimize |
||
| 184 | * |
||
| 185 | * @param array $fromPathString |
||
| 186 | * @param array $toPathString |
||
| 187 | */ |
||
| 188 | public function moveSubtreeNodes(array $fromPathString, array $toPathString) |
||
| 198 | |||
| 199 | /** |
||
| 200 | * Updated subtree modification time for all nodes on path. |
||
| 201 | * |
||
| 202 | * @param string $pathString |
||
| 203 | * @param int|null $timestamp |
||
| 204 | */ |
||
| 205 | public function updateSubtreeModificationTime($pathString, $timestamp = null) |
||
| 215 | |||
| 216 | /** |
||
| 217 | * Update node assignment table. |
||
| 218 | * |
||
| 219 | * @param int $contentObjectId |
||
| 220 | * @param int $oldParent |
||
| 221 | * @param int $newParent |
||
| 222 | * @param int $opcode |
||
| 223 | */ |
||
| 224 | public function updateNodeAssignment($contentObjectId, $oldParent, $newParent, $opcode) |
||
| 234 | |||
| 235 | /** |
||
| 236 | * Create locations from node assignments. |
||
| 237 | * |
||
| 238 | * Convert existing node assignments into real locations. |
||
| 239 | * |
||
| 240 | * @param mixed $contentId |
||
| 241 | * @param mixed $versionNo |
||
| 242 | */ |
||
| 243 | public function createLocationsFromNodeAssignments($contentId, $versionNo) |
||
| 253 | |||
| 254 | /** |
||
| 255 | * Updates all Locations of content identified with $contentId with $versionNo. |
||
| 256 | * |
||
| 257 | * @param mixed $contentId |
||
| 258 | * @param mixed $versionNo |
||
| 259 | */ |
||
| 260 | public function updateLocationsContentVersionNo($contentId, $versionNo) |
||
| 270 | |||
| 271 | /** |
||
| 272 | * Sets a location to be hidden, and it self + all children to invisible. |
||
| 273 | * |
||
| 274 | * @param string $pathString |
||
| 275 | */ |
||
| 276 | public function hideSubtree($pathString) |
||
| 286 | |||
| 287 | /** |
||
| 288 | * Sets a location to be unhidden, and self + children to visible unless a parent is hiding the tree. |
||
| 289 | * If not make sure only children down to first hidden node is marked visible. |
||
| 290 | * |
||
| 291 | * @param string $pathString |
||
| 292 | */ |
||
| 293 | public function unHideSubtree($pathString) |
||
| 303 | |||
| 304 | /** |
||
| 305 | * Swaps the content object being pointed to by a location object. |
||
| 306 | * |
||
| 307 | * Make the location identified by $locationId1 refer to the Content |
||
| 308 | * referred to by $locationId2 and vice versa. |
||
| 309 | * |
||
| 310 | * @param mixed $locationId1 |
||
| 311 | * @param mixed $locationId2 |
||
| 312 | * |
||
| 313 | * @return bool |
||
| 314 | */ |
||
| 315 | public function swap($locationId1, $locationId2) |
||
| 325 | |||
| 326 | /** |
||
| 327 | * Creates a new location in given $parentNode. |
||
| 328 | * |
||
| 329 | * @param \eZ\Publish\SPI\Persistence\Content\Location\CreateStruct $createStruct |
||
| 330 | * @param array $parentNode |
||
| 331 | * |
||
| 332 | * @return \eZ\Publish\SPI\Persistence\Content\Location |
||
| 333 | */ |
||
| 334 | public function create(CreateStruct $createStruct, array $parentNode) |
||
| 344 | |||
| 345 | /** |
||
| 346 | * Create an entry in the node assignment table. |
||
| 347 | * |
||
| 348 | * @param \eZ\Publish\SPI\Persistence\Content\Location\CreateStruct $createStruct |
||
| 349 | * @param mixed $parentNodeId |
||
| 350 | * @param int $type |
||
| 351 | */ |
||
| 352 | public function createNodeAssignment(CreateStruct $createStruct, $parentNodeId, $type = self::NODE_ASSIGNMENT_OP_CODE_CREATE_NOP) |
||
| 362 | |||
| 363 | /** |
||
| 364 | * Deletes node assignment for given $contentId and $versionNo. |
||
| 365 | * |
||
| 366 | * @param int $contentId |
||
| 367 | * @param int $versionNo |
||
| 368 | */ |
||
| 369 | public function deleteNodeAssignment($contentId, $versionNo = null) |
||
| 379 | |||
| 380 | /** |
||
| 381 | * Updates an existing location. |
||
| 382 | * |
||
| 383 | * Will not throw anything if location id is invalid or no entries are affected. |
||
| 384 | * |
||
| 385 | * @param \eZ\Publish\SPI\Persistence\Content\Location\UpdateStruct $location |
||
| 386 | * @param int $locationId |
||
| 387 | */ |
||
| 388 | public function update(UpdateStruct $location, $locationId) |
||
| 398 | |||
| 399 | /** |
||
| 400 | * Updates path identification string for given $locationId. |
||
| 401 | * |
||
| 402 | * @param mixed $locationId |
||
| 403 | * @param mixed $parentLocationId |
||
| 404 | * @param string $text |
||
| 405 | */ |
||
| 406 | public function updatePathIdentificationString($locationId, $parentLocationId, $text) |
||
| 416 | |||
| 417 | /** |
||
| 418 | * Deletes ezcontentobject_tree row for given $locationId (node_id). |
||
| 419 | * |
||
| 420 | * @param mixed $locationId |
||
| 421 | */ |
||
| 422 | public function removeLocation($locationId) |
||
| 432 | |||
| 433 | /** |
||
| 434 | * Returns id of the next in line node to be set as a new main node. |
||
| 435 | * |
||
| 436 | * This returns lowest node id for content identified by $contentId, and not of |
||
| 437 | * the node identified by given $locationId (current main node). |
||
| 438 | * Assumes that content has more than one location. |
||
| 439 | * |
||
| 440 | * @param mixed $contentId |
||
| 441 | * @param mixed $locationId |
||
| 442 | * |
||
| 443 | * @return array |
||
| 444 | */ |
||
| 445 | public function getFallbackMainNodeData($contentId, $locationId) |
||
| 455 | |||
| 456 | /** |
||
| 457 | * Sends a single location identified by given $locationId to the trash. |
||
| 458 | * |
||
| 459 | * The associated content object is left untouched. |
||
| 460 | * |
||
| 461 | * @param mixed $locationId |
||
| 462 | * |
||
| 463 | * @return bool |
||
| 464 | */ |
||
| 465 | public function trashLocation($locationId) |
||
| 475 | |||
| 476 | /** |
||
| 477 | * Returns a trashed location to normal state. |
||
| 478 | * |
||
| 479 | * Recreates the originally trashed location in the new position. If no new |
||
| 480 | * position has been specified, it will be tried to re-create the location |
||
| 481 | * at the old position. If this is not possible ( because the old location |
||
| 482 | * does not exist any more) and exception is thrown. |
||
| 483 | * |
||
| 484 | * @param mixed $locationId |
||
| 485 | * @param mixed $newParentId |
||
| 486 | * |
||
| 487 | * @return \eZ\Publish\SPI\Persistence\Content\Location |
||
| 488 | */ |
||
| 489 | public function untrashLocation($locationId, $newParentId = null) |
||
| 499 | |||
| 500 | /** |
||
| 501 | * Loads trash data specified by location ID. |
||
| 502 | * |
||
| 503 | * @param mixed $locationId |
||
| 504 | * |
||
| 505 | * @return array |
||
| 506 | */ |
||
| 507 | public function loadTrashByLocation($locationId) |
||
| 517 | |||
| 518 | /** |
||
| 519 | * Loads trash data specified by content ID. |
||
| 520 | * |
||
| 521 | * @param mixed $contentId |
||
| 522 | * |
||
| 523 | * @return array |
||
| 524 | */ |
||
| 525 | View Code Duplication | public function loadTrashByContent($contentId) |
|
| 535 | |||
| 536 | /** |
||
| 537 | * Removes every entries in the trash. |
||
| 538 | * Will NOT remove associated content objects nor attributes. |
||
| 539 | * |
||
| 540 | * Basically truncates ezcontentobject_trash table. |
||
| 541 | */ |
||
| 542 | public function cleanupTrash() |
||
| 552 | |||
| 553 | /** |
||
| 554 | * Lists trashed items. |
||
| 555 | * Returns entries from ezcontentobject_trash. |
||
| 556 | * |
||
| 557 | * @param int $offset |
||
| 558 | * @param int $limit |
||
| 559 | * @param array $sort |
||
| 560 | * |
||
| 561 | * @return array |
||
| 562 | */ |
||
| 563 | View Code Duplication | public function listTrashed($offset, $limit, array $sort = null) |
|
| 573 | |||
| 574 | /** |
||
| 575 | * Removes trashed element identified by $id from trash. |
||
| 576 | * Will NOT remove associated content object nor attributes. |
||
| 577 | * |
||
| 578 | * @param int $id The trashed location Id |
||
| 579 | */ |
||
| 580 | public function removeElementFromTrash($id) |
||
| 590 | |||
| 591 | /** |
||
| 592 | * Set section on all content objects in the subtree. |
||
| 593 | * |
||
| 594 | * @param mixed $pathString |
||
| 595 | * @param mixed $sectionId |
||
| 596 | * |
||
| 597 | * @return bool |
||
| 598 | */ |
||
| 599 | public function setSectionForSubtree($pathString, $sectionId) |
||
| 609 | |||
| 610 | /** |
||
| 611 | * Returns how many locations given content object identified by $contentId has. |
||
| 612 | * |
||
| 613 | * @param int $contentId |
||
| 614 | * |
||
| 615 | * @return int |
||
| 616 | */ |
||
| 617 | public function countLocationsByContentId($contentId) |
||
| 627 | |||
| 628 | /** |
||
| 629 | * Changes main location of content identified by given $contentId to location identified by given $locationId. |
||
| 630 | * |
||
| 631 | * Updates ezcontentobject_tree table for the given $contentId and eznode_assignment table for the given |
||
| 632 | * $contentId, $parentLocationId and $versionNo |
||
| 633 | * |
||
| 634 | * @param mixed $contentId |
||
| 635 | * @param mixed $locationId |
||
| 636 | * @param mixed $versionNo version number, needed to update eznode_assignment table |
||
| 637 | * @param mixed $parentLocationId parent location of location identified by $locationId, needed to update |
||
| 638 | * eznode_assignment table |
||
| 639 | */ |
||
| 640 | public function changeMainLocation($contentId, $locationId, $versionNo, $parentLocationId) |
||
| 650 | } |
||
| 651 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: