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 Validator 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 Validator, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace Limoncello\Flute\Validation; |
||
| 42 | class Validator extends BaseValidator implements JsonApiValidatorInterface |
||
| 43 | { |
||
| 44 | use RelationshipsTrait; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * Namespace for string resources. |
||
| 48 | */ |
||
| 49 | const RESOURCES_NAMESPACE = 'Limoncello.Flute.Validation'; |
||
| 50 | |||
| 51 | /** Rule description index */ |
||
| 52 | const RULE_INDEX = 0; |
||
| 53 | |||
| 54 | /** Rule description index */ |
||
| 55 | const RULE_ATTRIBUTES = self::RULE_INDEX + 1; |
||
| 56 | |||
| 57 | /** Rule description index */ |
||
| 58 | const RULE_TO_ONE = self::RULE_ATTRIBUTES + 1; |
||
| 59 | |||
| 60 | /** Rule description index */ |
||
| 61 | const RULE_TO_MANY = self::RULE_TO_ONE + 1; |
||
| 62 | |||
| 63 | /** Rule description index */ |
||
| 64 | const RULE_UNLISTED_ATTRIBUTE = self::RULE_TO_MANY + 1; |
||
| 65 | |||
| 66 | /** Rule description index */ |
||
| 67 | const RULE_UNLISTED_RELATIONSHIP = self::RULE_UNLISTED_ATTRIBUTE + 1; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * @var ContainerInterface |
||
| 71 | */ |
||
| 72 | private $container; |
||
| 73 | |||
| 74 | /** |
||
| 75 | * @var int |
||
| 76 | */ |
||
| 77 | private $errorStatus; |
||
| 78 | |||
| 79 | /** |
||
| 80 | * @var ContextStorageInterface |
||
| 81 | */ |
||
| 82 | private $contextStorage; |
||
| 83 | |||
| 84 | /** |
||
| 85 | * @var JsonApiErrorCollection |
||
| 86 | */ |
||
| 87 | private $jsonApiErrors; |
||
| 88 | |||
| 89 | /** |
||
| 90 | * @var array |
||
| 91 | */ |
||
| 92 | private $blocks; |
||
| 93 | |||
| 94 | /** |
||
| 95 | * @var array |
||
| 96 | */ |
||
| 97 | private $idRule; |
||
| 98 | |||
| 99 | /** |
||
| 100 | * @var array |
||
| 101 | */ |
||
| 102 | private $typeRule; |
||
| 103 | |||
| 104 | /** |
||
| 105 | * @var int[] |
||
| 106 | */ |
||
| 107 | private $attributeRules; |
||
| 108 | |||
| 109 | /** |
||
| 110 | * @var int[] |
||
| 111 | */ |
||
| 112 | private $toOneRules; |
||
| 113 | |||
| 114 | /** |
||
| 115 | * @var int[] |
||
| 116 | */ |
||
| 117 | private $toManyRules; |
||
| 118 | |||
| 119 | /** |
||
| 120 | * @var bool |
||
| 121 | */ |
||
| 122 | private $isIgnoreUnknowns; |
||
| 123 | |||
| 124 | /** |
||
| 125 | * @var FormatterInterface|null |
||
| 126 | */ |
||
| 127 | private $messageFormatter; |
||
| 128 | |||
| 129 | /** |
||
| 130 | * @param string $name |
||
| 131 | * @param array $data |
||
| 132 | * @param ContainerInterface $container |
||
| 133 | * @param int $errorStatus |
||
| 134 | * |
||
| 135 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 136 | */ |
||
| 137 | public function __construct( |
||
| 158 | |||
| 159 | /** |
||
| 160 | * @inheritdoc |
||
| 161 | */ |
||
| 162 | public function assert($jsonData): JsonApiValidatorInterface |
||
| 170 | |||
| 171 | /** |
||
| 172 | * @inheritdoc |
||
| 173 | */ |
||
| 174 | public function validate($input): bool |
||
| 188 | |||
| 189 | /** |
||
| 190 | * @inheritdoc |
||
| 191 | */ |
||
| 192 | public function getJsonApiErrors(): array |
||
| 196 | |||
| 197 | /** |
||
| 198 | * @inheritdoc |
||
| 199 | */ |
||
| 200 | public function getJsonApiCaptures(): array |
||
| 204 | |||
| 205 | /** |
||
| 206 | * @return BaseValidator |
||
| 207 | */ |
||
| 208 | protected function resetAggregators(): BaseValidator |
||
| 217 | |||
| 218 | /** |
||
| 219 | * @param array $jsonData |
||
| 220 | * |
||
| 221 | * @return self |
||
| 222 | * |
||
| 223 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 224 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 225 | */ |
||
| 226 | private function validateType(array $jsonData): self |
||
| 259 | |||
| 260 | /** |
||
| 261 | * @param array $jsonData |
||
| 262 | * |
||
| 263 | * @return self |
||
| 264 | * |
||
| 265 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 266 | */ |
||
| 267 | private function validateId(array $jsonData): self |
||
| 296 | |||
| 297 | /** |
||
| 298 | * @param array $jsonData |
||
| 299 | * |
||
| 300 | * @return self |
||
| 301 | * |
||
| 302 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 303 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 304 | */ |
||
| 305 | private function validateAttributes(array $jsonData): self |
||
| 346 | |||
| 347 | /** |
||
| 348 | * @param array $jsonData |
||
| 349 | * |
||
| 350 | * @return self |
||
| 351 | * |
||
| 352 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 353 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 354 | */ |
||
| 355 | private function validateRelationships(array $jsonData): self |
||
| 410 | |||
| 411 | /** |
||
| 412 | * @param int $index |
||
| 413 | * @param string $name |
||
| 414 | * @param mixed $mightBeRelationship |
||
| 415 | * |
||
| 416 | * @return void |
||
| 417 | * |
||
| 418 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 419 | */ |
||
| 420 | private function validateAsToOneRelationship(int $index, string $name, $mightBeRelationship): void |
||
| 434 | |||
| 435 | /** |
||
| 436 | * @param int $index |
||
| 437 | * @param string $name |
||
| 438 | * @param mixed $mightBeRelationship |
||
| 439 | * |
||
| 440 | * @return void |
||
| 441 | * |
||
| 442 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 443 | */ |
||
| 444 | private function validateAsToManyRelationship(int $index, string $name, $mightBeRelationship): void |
||
| 474 | |||
| 475 | /** |
||
| 476 | * @param mixed $data |
||
| 477 | * |
||
| 478 | * @return array|null|false Either `array` ($type => $id), or `null`, or `false` on error. |
||
| 479 | * |
||
| 480 | * @SuppressWarnings(PHPMD.ElseExpression) |
||
| 481 | */ |
||
| 482 | private function parseSingleRelationship($data) |
||
| 499 | |||
| 500 | /** |
||
| 501 | * Re-initializes internal aggregators for captures, errors, etc. |
||
| 502 | */ |
||
| 503 | private function reInitAggregatorsIfNeeded(): void |
||
| 507 | |||
| 508 | /** |
||
| 509 | * @param mixed $input |
||
| 510 | * @param int $index |
||
| 511 | * |
||
| 512 | * @return void |
||
| 513 | * |
||
| 514 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 515 | */ |
||
| 516 | private function executeBlock($input, int $index): void |
||
| 527 | |||
| 528 | /** |
||
| 529 | * @param array $indexes |
||
| 530 | * |
||
| 531 | * @return void |
||
| 532 | * |
||
| 533 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 534 | */ |
||
| 535 | private function executeStarts(array $indexes): void |
||
| 539 | |||
| 540 | /** |
||
| 541 | * @param array $indexes |
||
| 542 | * |
||
| 543 | * @return void |
||
| 544 | * |
||
| 545 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 546 | */ |
||
| 547 | private function executeEnds(array $indexes): void |
||
| 551 | |||
| 552 | /** |
||
| 553 | * @param ErrorInterface $error |
||
| 554 | * |
||
| 555 | * @return string |
||
| 556 | */ |
||
| 557 | View Code Duplication | private function getMessage(ErrorInterface $error): string |
|
| 565 | |||
| 566 | /** |
||
| 567 | * @return array |
||
| 568 | */ |
||
| 569 | protected function getIdRule(): array |
||
| 573 | |||
| 574 | /** |
||
| 575 | * @return array |
||
| 576 | */ |
||
| 577 | protected function getTypeRule(): array |
||
| 581 | |||
| 582 | /** |
||
| 583 | * @return ContextStorageInterface |
||
| 584 | */ |
||
| 585 | protected function getContextStorage(): ContextStorageInterface |
||
| 589 | |||
| 590 | /** |
||
| 591 | * @return ContextStorageInterface |
||
| 592 | */ |
||
| 593 | protected function createContextStorage(): ContextStorageInterface |
||
| 597 | |||
| 598 | /** |
||
| 599 | * @return JsonApiErrorCollection |
||
| 600 | */ |
||
| 601 | protected function getJsonApiErrorCollection(): JsonApiErrorCollection |
||
| 605 | |||
| 606 | /** |
||
| 607 | * @return JsonApiErrorCollection |
||
| 608 | */ |
||
| 609 | protected function createJsonApiErrors(): JsonApiErrorCollection |
||
| 613 | |||
| 614 | /** |
||
| 615 | * @return ContainerInterface |
||
| 616 | */ |
||
| 617 | protected function getContainer(): ContainerInterface |
||
| 621 | |||
| 622 | /** |
||
| 623 | * @return int |
||
| 624 | */ |
||
| 625 | protected function getErrorStatus(): int |
||
| 629 | |||
| 630 | /** |
||
| 631 | * @return bool |
||
| 632 | */ |
||
| 633 | protected function isIgnoreUnknowns(): bool |
||
| 637 | |||
| 638 | /** |
||
| 639 | * @return Validator |
||
| 640 | */ |
||
| 641 | protected function enableIgnoreUnknowns(): self |
||
| 647 | |||
| 648 | /** |
||
| 649 | * @return Validator |
||
| 650 | */ |
||
| 651 | protected function disableIgnoreUnknowns(): self |
||
| 657 | |||
| 658 | /** |
||
| 659 | * @param array $rules |
||
| 660 | * |
||
| 661 | * @return self |
||
| 662 | */ |
||
| 663 | private function setAttributeRules(array $rules): self |
||
| 671 | |||
| 672 | /** |
||
| 673 | * @param array $rules |
||
| 674 | * |
||
| 675 | * @return self |
||
| 676 | */ |
||
| 677 | private function setToOneIndexes(array $rules): self |
||
| 685 | |||
| 686 | /** |
||
| 687 | * @param array $rules |
||
| 688 | * |
||
| 689 | * @return self |
||
| 690 | */ |
||
| 691 | private function setToManyIndexes(array $rules): self |
||
| 699 | |||
| 700 | /** |
||
| 701 | * @return int[] |
||
| 702 | */ |
||
| 703 | protected function getAttributeRules(): array |
||
| 707 | |||
| 708 | /** |
||
| 709 | * @return int[] |
||
| 710 | */ |
||
| 711 | protected function getToOneRules(): array |
||
| 715 | |||
| 716 | /** |
||
| 717 | * @return int[] |
||
| 718 | */ |
||
| 719 | protected function getToManyRules(): array |
||
| 723 | |||
| 724 | /** |
||
| 725 | * @return array |
||
| 726 | */ |
||
| 727 | private function getBlocks(): array |
||
| 731 | |||
| 732 | /** |
||
| 733 | * @return FormatterInterface |
||
| 734 | */ |
||
| 735 | View Code Duplication | protected function getMessageFormatter(): FormatterInterface |
|
| 745 | |||
| 746 | /** |
||
| 747 | * @param string $name |
||
| 748 | * |
||
| 749 | * @return int |
||
| 750 | * |
||
| 751 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 752 | */ |
||
| 753 | private function getAttributeIndex(string $name): int |
||
| 760 | |||
| 761 | /** |
||
| 762 | * @param string $name |
||
| 763 | * |
||
| 764 | * @return bool |
||
| 765 | * |
||
| 766 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 767 | */ |
||
| 768 | private function hasAttributeIndex(string $name): bool |
||
| 775 | |||
| 776 | /** |
||
| 777 | * @param int $messageId |
||
| 778 | * @param array $args |
||
| 779 | * |
||
| 780 | * @return string |
||
| 781 | */ |
||
| 782 | private function formatMessage(int $messageId, array $args = []): string |
||
| 788 | |||
| 789 | /** |
||
| 790 | * @param array $rules |
||
| 791 | * |
||
| 792 | * @return bool |
||
| 793 | * |
||
| 794 | * @SuppressWarnings(PHPMD.StaticAccess) |
||
| 795 | */ |
||
| 796 | private function debugCheckIndexesExist(array $rules): bool |
||
| 812 | } |
||
| 813 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.