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 FleetValidator 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 FleetValidator, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 6 | class FleetValidator { |
||
| 7 | /** |
||
| 8 | * @var Fleet $fleet |
||
| 9 | */ |
||
| 10 | protected $fleet; |
||
| 11 | |||
| 12 | protected static $conditionsGlobal = array( |
||
| 13 | // Cheap checks - class Fleet already have all this info internally |
||
| 14 | 'checkSpeedPercentOld' => FLIGHT_FLEET_SPEED_WRONG, |
||
| 15 | 'checkTargetInUniverse' => FLIGHT_VECTOR_BEYOND_UNIVERSE, |
||
| 16 | 'checkTargetNotSource' => FLIGHT_VECTOR_SAME_SOURCE, |
||
| 17 | 'checkSenderNoVacation' => FLIGHT_PLAYER_VACATION_OWN, // tODO |
||
| 18 | 'checkTargetNoVacation' => FLIGHT_PLAYER_VACATION, |
||
| 19 | 'checkFleetNotEmpty' => FLIGHT_SHIPS_NO_SHIPS, |
||
| 20 | // 'checkUnitsPositive' => FLIGHT_SHIPS_NEGATIVE, // Unused - 'cause it's not allowed to put negative units into Unit class |
||
|
|
|||
| 21 | // 'checkOnlyFleetUnits' => FLIGHT_SHIPS_UNIT_WRONG, // Unused - 'cause it's only possible to pass to fleet SHIP or RESOURCE |
||
| 22 | 'checkOnlyFlyingUnits' => FLIGHT_SHIPS_UNMOVABLE, |
||
| 23 | 'checkResourcesPositive' => FLIGHT_RESOURCES_NEGATIVE, |
||
| 24 | 'checkNotTooFar' => FLIGHT_FLEET_TOO_FAR, |
||
| 25 | 'checkEnoughCapacity' => FLIGHT_FLEET_OVERLOAD, |
||
| 26 | |||
| 27 | // checkMissionAllowed |
||
| 28 | // consumptionIsNegative ?????? |
||
| 29 | |||
| 30 | // Medium checks - currently requires access to DB but potentially doesn't |
||
| 31 | 'checkSourceEnoughShips' => FLIGHT_SHIPS_NOT_ENOUGH, |
||
| 32 | 'checkSourceEnoughFuel' => FLIGHT_RESOURCES_FUEL_NOT_ENOUGH, |
||
| 33 | 'checkSourceEnoughResources' => FLIGHT_RESOURCES_NOT_ENOUGH, |
||
| 34 | |||
| 35 | 'checkMultiAccountNot' => FLIGHT_PLAYER_SAME_IP, |
||
| 36 | |||
| 37 | // Heavy checks - will absolutely require DB access |
||
| 38 | 'checkEnoughFleetSlots' => FLIGHT_FLEET_NO_SLOTS, |
||
| 39 | |||
| 40 | // TODO - THIS CHECKS SHOULD BE ADDED IN UNIT_CAPTAIN MODULE! |
||
| 41 | 'checkCaptainSent' => array( |
||
| 42 | true => array( |
||
| 43 | 'checkCaptainExists' => FLIGHT_CAPTAIN_NOT_HIRED, |
||
| 44 | 'checkCaptainOnPlanetType' => FLIGHT_CAPTAIN_ALREADY_FLYING, |
||
| 45 | 'checkCaptainOnPlanetSource' => FLIGHT_CAPTAIN_ON_OTHER_PLANET, |
||
| 46 | 'checkCaptainNotRelocating' => FLIGHT_CAPTAIN_RELOCATE_LOCK, |
||
| 47 | ), |
||
| 48 | ), |
||
| 49 | ); |
||
| 50 | |||
| 51 | |||
| 52 | /** |
||
| 53 | * FleetValidator constructor. |
||
| 54 | * |
||
| 55 | * @param Fleet $fleet |
||
| 56 | */ |
||
| 57 | public function __construct($fleet) { |
||
| 60 | |||
| 61 | /** |
||
| 62 | * |
||
| 63 | */ |
||
| 64 | public function validateGlobals() { |
||
| 92 | |||
| 93 | /** |
||
| 94 | * |
||
| 95 | */ |
||
| 96 | public function validate() { |
||
| 125 | |||
| 126 | /** |
||
| 127 | * @param array $checklist |
||
| 128 | * |
||
| 129 | * @throws Exception |
||
| 130 | */ |
||
| 131 | public function checkMissionRestrictions($checklist, $exception = true) { |
||
| 169 | |||
| 170 | |||
| 171 | /** |
||
| 172 | * @throws Exception |
||
| 173 | */ |
||
| 174 | protected function restrict2ToAllowedMissions() { |
||
| 179 | |||
| 180 | /** |
||
| 181 | * @throws Exception |
||
| 182 | */ |
||
| 183 | protected function restrict2ToAllowedPlanetTypes() { |
||
| 188 | |||
| 189 | |||
| 190 | /** |
||
| 191 | * @return bool |
||
| 192 | */ |
||
| 193 | protected function checkSpeedPercentOld() { |
||
| 196 | |||
| 197 | /** |
||
| 198 | * @return bool |
||
| 199 | */ |
||
| 200 | protected function checkSenderNoVacation() { |
||
| 203 | |||
| 204 | /** |
||
| 205 | * @return bool |
||
| 206 | */ |
||
| 207 | protected function checkTargetNoVacation() { |
||
| 210 | |||
| 211 | /** |
||
| 212 | * @return bool |
||
| 213 | */ |
||
| 214 | protected function checkMultiAccountNot() { |
||
| 217 | |||
| 218 | /** |
||
| 219 | * @return bool |
||
| 220 | */ |
||
| 221 | protected function checkTargetNotSource() { |
||
| 224 | |||
| 225 | /** |
||
| 226 | * @return bool |
||
| 227 | */ |
||
| 228 | protected function checkTargetInUniverse() { |
||
| 231 | |||
| 232 | /** |
||
| 233 | * @return bool |
||
| 234 | */ |
||
| 235 | protected function checkUnitsPositive() { |
||
| 238 | |||
| 239 | /** |
||
| 240 | * @return bool |
||
| 241 | */ |
||
| 242 | protected function checkOnlyFleetUnits() { |
||
| 245 | |||
| 246 | /** |
||
| 247 | * @return bool |
||
| 248 | */ |
||
| 249 | protected function checkOnlyFlyingUnits() { |
||
| 252 | |||
| 253 | /** |
||
| 254 | * @return bool |
||
| 255 | */ |
||
| 256 | protected function checkEnoughFleetSlots() { |
||
| 259 | |||
| 260 | |||
| 261 | /** |
||
| 262 | * @return bool |
||
| 263 | */ |
||
| 264 | protected function checkEnoughCapacity($includeResources = true) { |
||
| 273 | |||
| 274 | /** |
||
| 275 | * @return bool |
||
| 276 | */ |
||
| 277 | protected function checkNotTooFar() { |
||
| 280 | |||
| 281 | /** |
||
| 282 | * @return bool |
||
| 283 | */ |
||
| 284 | protected function checkDebrisExists() { |
||
| 287 | |||
| 288 | |||
| 289 | |||
| 290 | |||
| 291 | |||
| 292 | |||
| 293 | |||
| 294 | |||
| 295 | |||
| 296 | |||
| 297 | |||
| 298 | |||
| 299 | // Resources checks |
||
| 300 | |||
| 301 | /** |
||
| 302 | * @return bool |
||
| 303 | */ |
||
| 304 | protected function checkResourcesPositive() { |
||
| 313 | |||
| 314 | /** |
||
| 315 | * @return bool |
||
| 316 | */ |
||
| 317 | protected function checkCargo() { |
||
| 320 | |||
| 321 | /** |
||
| 322 | * @return bool |
||
| 323 | */ |
||
| 324 | protected function checkSourceEnoughFuel() { |
||
| 329 | |||
| 330 | /** |
||
| 331 | * @return bool |
||
| 332 | */ |
||
| 333 | protected function checkSourceEnoughResources() { |
||
| 344 | |||
| 345 | |||
| 346 | |||
| 347 | |||
| 348 | |||
| 349 | |||
| 350 | |||
| 351 | |||
| 352 | |||
| 353 | |||
| 354 | |||
| 355 | |||
| 356 | |||
| 357 | |||
| 358 | |||
| 359 | |||
| 360 | |||
| 361 | |||
| 362 | |||
| 363 | |||
| 364 | |||
| 365 | |||
| 366 | |||
| 367 | |||
| 368 | |||
| 369 | // Target vector checks (????????) |
||
| 370 | |||
| 371 | /** |
||
| 372 | * @return bool |
||
| 373 | */ |
||
| 374 | protected function checkKnownSpace() { |
||
| 377 | |||
| 378 | /** |
||
| 379 | * @return bool |
||
| 380 | */ |
||
| 381 | protected function checkUnKnownSpace() { |
||
| 384 | |||
| 385 | /** |
||
| 386 | * @return bool |
||
| 387 | */ |
||
| 388 | protected function checkTargetExists() { |
||
| 391 | |||
| 392 | /** |
||
| 393 | * @return bool |
||
| 394 | */ |
||
| 395 | protected function checkTargetNotExists() { |
||
| 398 | |||
| 399 | /** |
||
| 400 | * @return bool |
||
| 401 | */ |
||
| 402 | protected function checkTargetIsPlanet() { |
||
| 405 | |||
| 406 | /** |
||
| 407 | * @return bool |
||
| 408 | */ |
||
| 409 | protected function checkTargetIsDebris() { |
||
| 412 | |||
| 413 | /** |
||
| 414 | * @return bool |
||
| 415 | */ |
||
| 416 | protected function checkTargetIsMoon() { |
||
| 419 | |||
| 420 | |||
| 421 | |||
| 422 | |||
| 423 | |||
| 424 | |||
| 425 | // Ships checks |
||
| 426 | |||
| 427 | /** |
||
| 428 | * @return bool |
||
| 429 | */ |
||
| 430 | protected function checkFleetNotEmpty() { |
||
| 433 | |||
| 434 | |||
| 435 | /** |
||
| 436 | * @return bool |
||
| 437 | */ |
||
| 438 | protected function checkSourceEnoughShips() { |
||
| 441 | |||
| 442 | |||
| 443 | /** |
||
| 444 | * @return bool |
||
| 445 | */ |
||
| 446 | protected function checkHaveColonizer() { |
||
| 450 | |||
| 451 | /** |
||
| 452 | * @return bool |
||
| 453 | */ |
||
| 454 | protected function checkHaveRecyclers() { |
||
| 462 | |||
| 463 | /** |
||
| 464 | * @return bool |
||
| 465 | */ |
||
| 466 | // TODO - used only as callable. Redo inversion |
||
| 467 | protected function checkSpiesOnly() { |
||
| 470 | |||
| 471 | /** |
||
| 472 | * @return bool |
||
| 473 | */ |
||
| 474 | protected function checkNotOnlySpies() { |
||
| 477 | |||
| 478 | /** |
||
| 479 | * @return bool |
||
| 480 | */ |
||
| 481 | protected function checkNoMissiles() { |
||
| 487 | |||
| 488 | |||
| 489 | /** |
||
| 490 | * @return bool |
||
| 491 | */ |
||
| 492 | protected function checkTargetOwn() { |
||
| 495 | |||
| 496 | /** |
||
| 497 | * @return bool |
||
| 498 | */ |
||
| 499 | protected function forceTargetOwn() { |
||
| 517 | |||
| 518 | protected function checkMissionPeaceful() { |
||
| 528 | |||
| 529 | /** |
||
| 530 | * @return bool |
||
| 531 | */ |
||
| 532 | protected function checkTargetOther() { |
||
| 535 | |||
| 536 | |||
| 537 | /** |
||
| 538 | * @return bool |
||
| 539 | */ |
||
| 540 | protected function alwaysFalse() { |
||
| 543 | |||
| 544 | |||
| 545 | /** |
||
| 546 | * @return bool |
||
| 547 | */ |
||
| 548 | protected function checkTargetAllyDeposit() { |
||
| 556 | |||
| 557 | /** |
||
| 558 | * @param int $missionType |
||
| 559 | * @param bool $exact |
||
| 560 | * |
||
| 561 | * @return bool |
||
| 562 | */ |
||
| 563 | protected function checkMission($missionType, $exact = false) { |
||
| 566 | |||
| 567 | |||
| 568 | /** |
||
| 569 | * Check mission type OR no mission - and limits available missions to this type if positive |
||
| 570 | * |
||
| 571 | * @param int $missionType |
||
| 572 | * |
||
| 573 | * @return bool |
||
| 574 | */ |
||
| 575 | protected function forceMission($missionType, $exact = false) { |
||
| 578 | |||
| 579 | protected function unsetMission($missionType, $result, $forceMission = false) { |
||
| 590 | |||
| 591 | /** |
||
| 592 | * @param string $name |
||
| 593 | * @param string $prefix |
||
| 594 | * |
||
| 595 | * @return int|false |
||
| 596 | * @throws Exception |
||
| 597 | */ |
||
| 598 | protected function checkMissionPrefix($name, $prefix) { |
||
| 612 | |||
| 613 | public function __call($name, $arguments) { |
||
| 629 | |||
| 630 | /** |
||
| 631 | * Just checks mission type |
||
| 632 | * |
||
| 633 | * @param int $missionType |
||
| 634 | * |
||
| 635 | * @return bool |
||
| 636 | */ |
||
| 637 | // TODO - obsolete ?? |
||
| 638 | protected function checkMissionExact($missionType) { |
||
| 641 | |||
| 642 | |||
| 643 | /** |
||
| 644 | * @return bool |
||
| 645 | */ |
||
| 646 | protected function checkNotEmptyMission() { |
||
| 649 | |||
| 650 | /** |
||
| 651 | * @return bool |
||
| 652 | */ |
||
| 653 | protected function checkRealFlight() { |
||
| 656 | |||
| 657 | |||
| 658 | /** |
||
| 659 | * @return bool |
||
| 660 | */ |
||
| 661 | protected function unsetMissionSpyComplex() { |
||
| 672 | |||
| 673 | |||
| 674 | /** |
||
| 675 | * @return bool |
||
| 676 | */ |
||
| 677 | protected function checkMissionExists() { |
||
| 680 | |||
| 681 | /** |
||
| 682 | * @return bool |
||
| 683 | */ |
||
| 684 | protected function checkMissionAllowed() { |
||
| 687 | |||
| 688 | /** |
||
| 689 | * @return bool |
||
| 690 | */ |
||
| 691 | protected function checkPlayerInactiveOrNotNoob() { |
||
| 697 | |||
| 698 | /** |
||
| 699 | * @return bool |
||
| 700 | */ |
||
| 701 | protected function checkTargetActive() { |
||
| 707 | |||
| 708 | /** |
||
| 709 | * @return bool |
||
| 710 | */ |
||
| 711 | // TODO - REDO MAIN FUNCTION |
||
| 712 | protected function checkTargetNotActive() { |
||
| 715 | |||
| 716 | |||
| 717 | /** |
||
| 718 | * @return bool |
||
| 719 | */ |
||
| 720 | protected function checkSameAlly() { |
||
| 723 | |||
| 724 | /** |
||
| 725 | * @return bool |
||
| 726 | */ |
||
| 727 | protected function checkTargetNoob() { |
||
| 745 | |||
| 746 | /** |
||
| 747 | * @return bool |
||
| 748 | */ |
||
| 749 | // TODO - REDO MAIN FUNCTION |
||
| 750 | protected function checkTargetNotNoob() { |
||
| 753 | |||
| 754 | |||
| 755 | /** |
||
| 756 | * @return bool |
||
| 757 | */ |
||
| 758 | protected function checkMissionHoldReal() { |
||
| 764 | |||
| 765 | /** |
||
| 766 | * @return bool |
||
| 767 | */ |
||
| 768 | protected function checkMissionHoldOnNotNoob() { |
||
| 776 | |||
| 777 | |||
| 778 | // Missiles |
||
| 779 | |||
| 780 | /** |
||
| 781 | * @return bool |
||
| 782 | */ |
||
| 783 | protected function checkOnlyAttackMissiles() { |
||
| 788 | |||
| 789 | /** |
||
| 790 | * @return bool |
||
| 791 | */ |
||
| 792 | protected function checkSiloLevel() { |
||
| 797 | |||
| 798 | /** |
||
| 799 | * @return bool |
||
| 800 | */ |
||
| 801 | protected function checkSameGalaxy() { |
||
| 804 | |||
| 805 | /** |
||
| 806 | * @return bool |
||
| 807 | */ |
||
| 808 | protected function checkMissileDistance() { |
||
| 811 | |||
| 812 | /** |
||
| 813 | * @return bool |
||
| 814 | */ |
||
| 815 | protected function checkMissileTarget() { |
||
| 818 | |||
| 819 | |||
| 820 | /** |
||
| 821 | * @return int |
||
| 822 | */ |
||
| 823 | protected function checkExpeditionsMax() { |
||
| 826 | |||
| 827 | /** |
||
| 828 | * @return bool |
||
| 829 | */ |
||
| 830 | protected function checkExpeditionsFree() { |
||
| 833 | |||
| 834 | /** |
||
| 835 | * @return bool |
||
| 836 | */ |
||
| 837 | protected function checkCaptainSent() { |
||
| 840 | |||
| 841 | /** |
||
| 842 | * @return bool |
||
| 843 | */ |
||
| 844 | protected function checkCaptainExists() { |
||
| 847 | |||
| 848 | /** |
||
| 849 | * @return bool |
||
| 850 | */ |
||
| 851 | protected function checkCaptainOnPlanetType() { |
||
| 854 | |||
| 855 | /** |
||
| 856 | * @return bool |
||
| 857 | */ |
||
| 858 | protected function checkCaptainOnPlanetSource() { |
||
| 861 | |||
| 862 | /** |
||
| 863 | * @return bool |
||
| 864 | */ |
||
| 865 | protected function checkCaptainNotRelocating() { |
||
| 874 | |||
| 875 | |||
| 876 | /** |
||
| 877 | * @return bool |
||
| 878 | */ |
||
| 879 | protected function checkMissionDestroyReal() { |
||
| 885 | |||
| 886 | /** |
||
| 887 | * @return bool |
||
| 888 | */ |
||
| 889 | protected function checkHaveReapers() { |
||
| 897 | |||
| 898 | |||
| 899 | /** |
||
| 900 | * @return bool |
||
| 901 | */ |
||
| 902 | protected function checkMissionACSReal() { |
||
| 908 | |||
| 909 | protected function checkACSInTime() { |
||
| 912 | |||
| 913 | |||
| 914 | protected function checkMissionRealAndSelected($missionType) { |
||
| 920 | |||
| 921 | protected function checkMissionResultAndUnset($missionType, $result, $forceMission = false) { |
||
| 926 | |||
| 927 | |||
| 928 | /** |
||
| 929 | * @return bool |
||
| 930 | */ |
||
| 931 | protected function checkMissionSpyPossibleAndReal() { |
||
| 938 | |||
| 939 | /** |
||
| 940 | * @return bool |
||
| 941 | */ |
||
| 942 | protected function checkMissionDestroyAndReal() { |
||
| 948 | |||
| 949 | /** |
||
| 950 | * @return bool |
||
| 951 | */ |
||
| 952 | protected function checkMissionHoldPossibleAndReal() { |
||
| 958 | |||
| 959 | /** |
||
| 960 | * @return bool |
||
| 961 | */ |
||
| 962 | protected function checkSpiesOnlyFriendlyRestrictsToRelocate() { |
||
| 971 | |||
| 972 | |||
| 973 | protected function checkFleetGroupACS() { |
||
| 984 | |||
| 985 | protected function checkACSNotEmpty() { |
||
| 988 | |||
| 989 | /** |
||
| 990 | * @return bool |
||
| 991 | */ |
||
| 992 | protected function checkACSInvited() { |
||
| 1002 | |||
| 1003 | /** |
||
| 1004 | * @return bool |
||
| 1005 | */ |
||
| 1006 | protected function checkMissionACSPossibleAndReal() { |
||
| 1013 | |||
| 1014 | /** |
||
| 1015 | * @return bool |
||
| 1016 | */ |
||
| 1017 | protected function checkMissionTransportPossibleAndReal() { |
||
| 1023 | |||
| 1024 | /** |
||
| 1025 | * @return bool |
||
| 1026 | */ |
||
| 1027 | protected function checkMissionTransportReal() { |
||
| 1033 | |||
| 1034 | |||
| 1035 | protected function checkBashingNotRestricted() { |
||
| 1038 | |||
| 1039 | protected function checkBashingBothAllies() { |
||
| 1042 | |||
| 1043 | protected function checkBashingAlliesHaveRelationWar() { |
||
| 1046 | |||
| 1047 | protected function checkBashingBothAlliesAndRelationWar() { |
||
| 1050 | |||
| 1051 | protected function checkBashingAlliesWarNoDelay() { |
||
| 1059 | |||
| 1060 | |||
| 1061 | protected function checkBashingNone() { |
||
| 1109 | |||
| 1110 | } |
||
| 1111 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.