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 PluginService 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 PluginService, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 32 | class PluginService |
||
| 33 | { |
||
| 34 | /** |
||
| 35 | * @var EccubeConfig |
||
| 36 | */ |
||
| 37 | protected $eccubeConfig; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * @var EntityManager |
||
| 41 | */ |
||
| 42 | protected $entityManager; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var PluginRepository |
||
| 46 | */ |
||
| 47 | protected $pluginRepository; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @var Application |
||
| 51 | */ |
||
| 52 | protected $app; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * @var EntityProxyService |
||
| 56 | */ |
||
| 57 | protected $entityProxyService; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * @var SchemaService |
||
| 61 | */ |
||
| 62 | protected $schemaService; |
||
| 63 | |||
| 64 | /** |
||
| 65 | * @var ComposerServiceInterface |
||
| 66 | */ |
||
| 67 | protected $composerService; |
||
| 68 | |||
| 69 | const VENDOR_NAME = 'ec-cube'; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Plugin type/library of ec-cube |
||
| 73 | */ |
||
| 74 | const ECCUBE_LIBRARY = 1; |
||
| 75 | |||
| 76 | /** |
||
| 77 | * Plugin type/library of other (except ec-cube) |
||
| 78 | */ |
||
| 79 | const OTHER_LIBRARY = 2; |
||
| 80 | |||
| 81 | /** |
||
| 82 | * @var string %kernel.project_dir% |
||
| 83 | */ |
||
| 84 | private $projectRoot; |
||
| 85 | |||
| 86 | /** |
||
| 87 | * @var string %kernel.environment% |
||
| 88 | */ |
||
| 89 | private $environment; |
||
| 90 | |||
| 91 | /** |
||
| 92 | * @var \Symfony\Component\DependencyInjection\ContainerInterface |
||
| 93 | */ |
||
| 94 | protected $container; |
||
| 95 | |||
| 96 | /** @var CacheUtil */ |
||
| 97 | protected $cacheUtil; |
||
| 98 | |||
| 99 | /** |
||
| 100 | * PluginService constructor. |
||
| 101 | * |
||
| 102 | * @param EntityManagerInterface $entityManager |
||
| 103 | * @param PluginRepository $pluginRepository |
||
| 104 | * @param EntityProxyService $entityProxyService |
||
| 105 | * @param SchemaService $schemaService |
||
| 106 | * @param EccubeConfig $eccubeConfig |
||
| 107 | * @param ContainerInterface $container |
||
| 108 | * @param CacheUtil $cacheUtil |
||
| 109 | * @param ComposerApiService $composerService |
||
| 110 | */ |
||
| 111 | public function __construct( |
||
| 132 | |||
| 133 | /** |
||
| 134 | * ファイル指定してのプラグインインストール |
||
| 135 | * |
||
| 136 | * @param string $path path to tar.gz/zip plugin file |
||
| 137 | * @param int $source |
||
| 138 | * |
||
| 139 | * @return boolean |
||
| 140 | * |
||
| 141 | * @throws PluginException |
||
| 142 | * @throws \Exception |
||
| 143 | */ |
||
| 144 | public function install($path, $source = 0) |
||
| 195 | |||
| 196 | // インストール事前処理 |
||
| 197 | public function preInstall() |
||
| 203 | |||
| 204 | // インストール事後処理 |
||
| 205 | public function postInstall($config, $source) |
||
| 237 | |||
| 238 | public function createTempDir() |
||
| 250 | |||
| 251 | public function deleteDirs($arr) |
||
| 260 | |||
| 261 | /** |
||
| 262 | * @param string $archive |
||
| 263 | * @param string $dir |
||
| 264 | * |
||
| 265 | * @throws PluginException |
||
| 266 | */ |
||
| 267 | public function unpackPluginArchive($archive, $dir) |
||
| 284 | |||
| 285 | /** |
||
| 286 | * @param $dir |
||
| 287 | * @param array $config_cache |
||
| 288 | * |
||
| 289 | * @throws PluginException |
||
| 290 | */ |
||
| 291 | public function checkPluginArchiveContent($dir, array $config_cache = []) |
||
| 328 | |||
| 329 | /** |
||
| 330 | * @param $pluginDir |
||
| 331 | * |
||
| 332 | * @return array |
||
| 333 | * |
||
| 334 | * @throws PluginException |
||
| 335 | */ |
||
| 336 | public function readConfig($pluginDir) |
||
| 362 | |||
| 363 | public function checkSymbolName($string) |
||
| 370 | |||
| 371 | /** |
||
| 372 | * @param string $path |
||
| 373 | */ |
||
| 374 | public function deleteFile($path) |
||
| 379 | |||
| 380 | public function checkSamePlugin($code) |
||
| 387 | |||
| 388 | public function calcPluginDir($name) |
||
| 392 | |||
| 393 | /** |
||
| 394 | * @param string $d |
||
| 395 | * |
||
| 396 | * @throws PluginException |
||
| 397 | */ |
||
| 398 | public function createPluginDir($d) |
||
| 405 | |||
| 406 | /** |
||
| 407 | * @param $meta |
||
| 408 | * @param int $source |
||
| 409 | * |
||
| 410 | * @return Plugin |
||
| 411 | * |
||
| 412 | * @throws PluginException |
||
| 413 | * @throws \Doctrine\DBAL\ConnectionException |
||
| 414 | */ |
||
| 415 | public function registerPlugin($meta, $source = 0) |
||
| 442 | |||
| 443 | /** |
||
| 444 | * @param $meta |
||
| 445 | * @param string $method |
||
| 446 | */ |
||
| 447 | public function callPluginManagerMethod($meta, $method) |
||
| 458 | |||
| 459 | /** |
||
| 460 | * @param Plugin $plugin |
||
| 461 | * @param bool $force |
||
| 462 | * |
||
| 463 | * @return bool |
||
| 464 | * |
||
| 465 | * @throws \Exception |
||
| 466 | */ |
||
| 467 | public function uninstall(Plugin $plugin, $force = true) |
||
| 492 | |||
| 493 | public function unregisterPlugin(Plugin $p) |
||
| 503 | |||
| 504 | public function disable(Plugin $plugin) |
||
| 508 | |||
| 509 | /** |
||
| 510 | * Proxyを再生成します. |
||
| 511 | * |
||
| 512 | * @param Plugin $plugin プラグイン |
||
| 513 | * @param boolean $temporary プラグインが無効状態でも一時的に生成するかどうか |
||
| 514 | * @param string|null $outputDir 出力先 |
||
| 515 | * |
||
| 516 | * @return array 生成されたファイルのパス |
||
| 517 | */ |
||
| 518 | private function regenerateProxy(Plugin $plugin, $temporary, $outputDir = null) |
||
| 551 | |||
| 552 | public function enable(Plugin $plugin, $enable = true) |
||
| 576 | |||
| 577 | /** |
||
| 578 | * Update plugin |
||
| 579 | * |
||
| 580 | * @param Plugin $plugin |
||
| 581 | * @param string $path |
||
| 582 | * |
||
| 583 | * @return bool |
||
| 584 | * |
||
| 585 | * @throws PluginException |
||
| 586 | * @throws \Exception |
||
| 587 | */ |
||
| 588 | public function update(Plugin $plugin, $path) |
||
| 630 | |||
| 631 | /** |
||
| 632 | * Update plugin |
||
| 633 | * |
||
| 634 | * @param Plugin $plugin |
||
| 635 | * @param array $meta Config data |
||
| 636 | * |
||
| 637 | * @throws \Exception |
||
| 638 | */ |
||
| 639 | public function updatePlugin(Plugin $plugin, $meta) |
||
| 656 | |||
| 657 | /** |
||
| 658 | * Get array require by plugin |
||
| 659 | * Todo: need define dependency plugin mechanism |
||
| 660 | * |
||
| 661 | * @param array|Plugin $plugin format as plugin from api |
||
| 662 | * |
||
| 663 | * @return array|mixed |
||
| 664 | * @throws PluginException |
||
| 665 | */ |
||
| 666 | public function getPluginRequired($plugin) |
||
| 679 | |||
| 680 | /** |
||
| 681 | * Get plugin information |
||
| 682 | * |
||
| 683 | * @param array $plugin |
||
| 684 | * |
||
| 685 | * @return array|null |
||
| 686 | */ |
||
| 687 | public function buildInfo($plugin) |
||
| 693 | |||
| 694 | /** |
||
| 695 | * Check support version |
||
| 696 | * |
||
| 697 | * @param $plugin |
||
| 698 | */ |
||
| 699 | public function supportedVersion(&$plugin) |
||
| 708 | |||
| 709 | /** |
||
| 710 | * Find the dependent plugins that need to be disabled |
||
| 711 | * |
||
| 712 | * @param string $pluginCode |
||
| 713 | * |
||
| 714 | * @return array plugin code |
||
| 715 | */ |
||
| 716 | public function findDependentPluginNeedDisable($pluginCode) |
||
| 720 | |||
| 721 | /** |
||
| 722 | * Find the other plugin that has requires on it. |
||
| 723 | * Check in both dtb_plugin table and <PluginCode>/composer.json |
||
| 724 | * |
||
| 725 | * @param string $pluginCode |
||
| 726 | * @param bool $enableOnly |
||
| 727 | * |
||
| 728 | * @return array plugin code |
||
| 729 | */ |
||
| 730 | public function findDependentPlugin($pluginCode, $enableOnly = false) |
||
| 762 | |||
| 763 | /** |
||
| 764 | * Get dependent plugin by code |
||
| 765 | * It's base on composer.json |
||
| 766 | * Return the plugin code and version in the format of the composer |
||
| 767 | * |
||
| 768 | * @param string $pluginCode |
||
| 769 | * @param int|null $libraryType |
||
| 770 | * self::ECCUBE_LIBRARY only return library/plugin of eccube |
||
| 771 | * self::OTHER_LIBRARY only return library/plugin of 3rd part ex: symfony, composer, ... |
||
| 772 | * default : return all library/plugin |
||
| 773 | * |
||
| 774 | * @return array format [packageName1 => version1, packageName2 => version2] |
||
| 775 | */ |
||
| 776 | public function getDependentByCode($pluginCode, $libraryType = null) |
||
| 805 | |||
| 806 | /** |
||
| 807 | * Format array dependent plugin to string |
||
| 808 | * It is used for commands. |
||
| 809 | * |
||
| 810 | * @param array $packages format [packageName1 => version1, packageName2 => version2] |
||
| 811 | * @param bool $getVersion |
||
| 812 | * |
||
| 813 | * @return string format if version=true: "packageName1:version1 packageName2:version2", if version=false: "packageName1 packageName2" |
||
| 814 | */ |
||
| 815 | public function parseToComposerCommand(array $packages, $getVersion = true) |
||
| 826 | |||
| 827 | /** |
||
| 828 | * リソースファイル等をコピー |
||
| 829 | * コピー元となるファイルの置き場所は固定であり、 |
||
| 830 | * [プラグインコード]/Resource/assets |
||
| 831 | * 配下に置かれているファイルが所定の位置へコピーされる |
||
| 832 | * |
||
| 833 | * @param string $pluginBaseDir |
||
| 834 | * @param $pluginCode |
||
| 835 | */ |
||
| 836 | public function copyAssets($pluginBaseDir, $pluginCode) |
||
| 846 | |||
| 847 | /** |
||
| 848 | * コピーしたリソースファイル等を削除 |
||
| 849 | * |
||
| 850 | * @param string $pluginCode |
||
| 851 | */ |
||
| 852 | public function removeAssets($pluginCode) |
||
| 862 | |||
| 863 | /** |
||
| 864 | * Is update |
||
| 865 | * |
||
| 866 | * @param string $pluginVersion |
||
| 867 | * @param string $remoteVersion |
||
| 868 | * |
||
| 869 | * @return boolean |
||
| 870 | */ |
||
| 871 | public function isUpdate($pluginVersion, $remoteVersion) |
||
| 875 | |||
| 876 | /** |
||
| 877 | * Plugin is exist check |
||
| 878 | * |
||
| 879 | * @param array $plugins get from api |
||
| 880 | * @param string $pluginCode |
||
| 881 | * |
||
| 882 | * @return false|int|string |
||
| 883 | */ |
||
| 884 | public function checkPluginExist($plugins, $pluginCode) |
||
| 894 | |||
| 895 | /** |
||
| 896 | * @param string $code |
||
| 897 | * |
||
| 898 | * @return bool |
||
| 899 | */ |
||
| 900 | private function isEnable($code) |
||
| 912 | } |
||
| 913 |
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 given class or a super-class is assigned to a property that is type hinted more strictly.
Either this assignment is in error or an instanceof check should be added for that assignment.