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.