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 BundleSyncHelper 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 BundleSyncHelper, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 37 | class BundleSyncHelper |
||
| 38 | { |
||
| 39 | /** |
||
| 40 | * @var ZikulaHttpKernelInterface |
||
| 41 | */ |
||
| 42 | private $kernel; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var ExtensionRepository |
||
| 46 | */ |
||
| 47 | private $extensionRepository; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @var ExtensionVarRepository |
||
| 51 | */ |
||
| 52 | private $extensionVarRepository; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * @var ExtensionDependencyRepository |
||
| 56 | */ |
||
| 57 | private $extensionDependencyRepository; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * @var TranslatorInterface |
||
| 61 | */ |
||
| 62 | private $translator; |
||
| 63 | |||
| 64 | /** |
||
| 65 | * @var EventDispatcherInterface |
||
| 66 | */ |
||
| 67 | private $dispatcher; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * @var ExtensionStateHelper |
||
| 71 | */ |
||
| 72 | private $extensionStateHelper; |
||
| 73 | |||
| 74 | /** |
||
| 75 | * @var BootstrapHelper |
||
| 76 | */ |
||
| 77 | private $bootstrapHelper; |
||
| 78 | |||
| 79 | /** |
||
| 80 | * @var ComposerValidationHelper |
||
| 81 | */ |
||
| 82 | private $composerValidationHelper; |
||
| 83 | |||
| 84 | /** |
||
| 85 | * @var SessionInterface |
||
| 86 | */ |
||
| 87 | protected $session; |
||
| 88 | |||
| 89 | /** |
||
| 90 | * BundleSyncHelper constructor. |
||
| 91 | * |
||
| 92 | * @param ZikulaHttpKernelInterface $kernel |
||
| 93 | * @param ExtensionRepository $extensionRepository |
||
| 94 | * @param ExtensionVarRepository $extensionVarRepository |
||
| 95 | * @param ExtensionDependencyRepository $extensionDependencyRepository |
||
| 96 | * @param TranslatorInterface $translator |
||
| 97 | * @param EventDispatcherInterface $dispatcher |
||
| 98 | * @param ExtensionStateHelper $extensionStateHelper |
||
| 99 | * @param ComposerValidationHelper $composerValidationHelper |
||
| 100 | * @param SessionInterface $session |
||
| 101 | */ |
||
| 102 | public function __construct( |
||
| 125 | |||
| 126 | /** |
||
| 127 | * Scan the file system for bundles. |
||
| 128 | * |
||
| 129 | * This function scans the file system for bundles and returns an array with all (potential) bundles found. |
||
| 130 | * |
||
| 131 | * @param array $directories |
||
| 132 | * @return array Thrown if the user doesn't have admin permissions over the bundle |
||
| 133 | * @throws \Exception |
||
| 134 | */ |
||
| 135 | public function scanForBundles(array $directories = []) |
||
| 136 | { |
||
| 137 | $directories = empty($directories) ? ['system', 'modules'] : $directories; |
||
| 138 | |||
| 139 | // sync the filesystem and the bundles table |
||
| 140 | $this->bootstrapHelper->load(); |
||
| 141 | |||
| 142 | // Get all bundles on filesystem |
||
| 143 | $bundles = []; |
||
| 144 | |||
| 145 | $scanner = new Scanner(); |
||
| 146 | $scanner->scan($directories, 5); |
||
| 147 | $newModules = $scanner->getModulesMetaData(); |
||
| 148 | |||
| 149 | // scan for all bundle-type bundles (psr-4) in either /system or /bundles |
||
| 150 | /** @var MetaData $bundleMetaData */ |
||
| 151 | foreach ($newModules as $name => $bundleMetaData) { |
||
| 152 | foreach ($bundleMetaData->getPsr4() as $ns => $path) { |
||
| 153 | $this->kernel->getAutoloader()->addPsr4($ns, $path); |
||
| 154 | } |
||
| 155 | |||
| 156 | $bundleClass = $bundleMetaData->getClass(); |
||
| 157 | |||
| 158 | /** @var $bundle \Zikula\Core\AbstractModule */ |
||
| 159 | $bundle = new $bundleClass(); |
||
| 160 | $bundleMetaData->setTranslator($this->translator); |
||
| 161 | $bundleMetaData->setDirectoryFromBundle($bundle); |
||
| 162 | $bundleVersionArray = $bundleMetaData->getFilteredVersionInfoArray(); |
||
| 163 | $bundleVersionArray['capabilities'] = serialize($bundleVersionArray['capabilities']); |
||
| 164 | $bundleVersionArray['securityschema'] = serialize($bundleVersionArray['securityschema']); |
||
| 165 | $bundleVersionArray['dependencies'] = serialize($bundleVersionArray['dependencies']); |
||
| 166 | |||
| 167 | $finder = new Finder(); |
||
| 168 | $finder->files()->in($bundle->getPath())->depth(0)->name('composer.json'); |
||
| 169 | foreach ($finder as $splFileInfo) { |
||
| 170 | // there will only be one loop here |
||
| 171 | $this->composerValidationHelper->check($splFileInfo); |
||
| 172 | if ($this->composerValidationHelper->isValid()) { |
||
| 173 | $bundles[$bundle->getName()] = $bundleVersionArray; |
||
| 174 | $bundles[$bundle->getName()]['oldnames'] = isset($bundleVersionArray['oldnames']) ? $bundleVersionArray['oldnames'] : ''; |
||
| 175 | View Code Duplication | } else { |
|
|
|
|||
| 176 | $this->session->getFlashBag()->add('error', $this->translator->__f('Cannot load %extension because the composer file is invalid.', ['%extension' => $bundle->getName()])); |
||
| 177 | foreach ($this->composerValidationHelper->getErrors() as $error) { |
||
| 178 | $this->session->getFlashBag()->add('error', $error); |
||
| 179 | } |
||
| 180 | } |
||
| 181 | } |
||
| 182 | } |
||
| 183 | |||
| 184 | $this->validate($bundles); |
||
| 185 | |||
| 186 | return $bundles; |
||
| 187 | } |
||
| 188 | |||
| 189 | /** |
||
| 190 | * Validate the extensions and ensure there are no duplicate names, displaynames or urls. |
||
| 191 | * |
||
| 192 | * @param array $extensions |
||
| 193 | * @throws FatalErrorException |
||
| 194 | */ |
||
| 195 | private function validate(array $extensions) |
||
| 196 | { |
||
| 197 | $modulenames = []; |
||
| 198 | $displaynames = []; |
||
| 199 | $urls = []; |
||
| 200 | |||
| 201 | // check for duplicate name, displayname or url |
||
| 202 | foreach ($extensions as $dir => $modInfo) { |
||
| 203 | $fields = ['name', 'displayname', 'url']; |
||
| 204 | foreach ($fields as $field) { |
||
| 205 | if (isset($modulenames[strtolower($modInfo[$field])])) { |
||
| 206 | throw new FatalErrorException($this->translator->__f('Fatal Error: Two extensions share the same %field. [%ext1%] and [%ext2%]', [ |
||
| 207 | '%field' => $field, |
||
| 208 | '%ext1%' => $modInfo['name'], |
||
| 209 | '%ext2%' => $modulenames[strtolower($modInfo['name'])] |
||
| 210 | ])); |
||
| 211 | } |
||
| 212 | } |
||
| 213 | |||
| 214 | $modulenames[strtolower($modInfo['name'])] = $dir; |
||
| 215 | $displaynames[strtolower($modInfo['displayname'])] = $dir; |
||
| 216 | $urls[strtolower($modInfo['url'])] = $dir; |
||
| 217 | } |
||
| 218 | } |
||
| 219 | |||
| 220 | /** |
||
| 221 | * Sync extensions in the filesystem and the database. |
||
| 222 | * @param array $extensionsFromFile |
||
| 223 | * @param bool $forceDefaults |
||
| 224 | * @return array $upgradedExtensions[<name>] = <version> |
||
| 225 | */ |
||
| 226 | public function syncExtensions(array $extensionsFromFile, $forceDefaults = false) |
||
| 246 | |||
| 247 | /** |
||
| 248 | * Sync extensions that are already in the Database. |
||
| 249 | * - update from old names |
||
| 250 | * - update compatibility |
||
| 251 | * - update user settings (or reset to defaults) |
||
| 252 | * - ensure current core compatibility |
||
| 253 | * |
||
| 254 | * @param array $extensionsFromFile |
||
| 255 | * @param array $extensionsFromDB |
||
| 256 | * @param bool $forceDefaults |
||
| 257 | */ |
||
| 258 | private function syncUpdatedExtensions(array $extensionsFromFile, array &$extensionsFromDB, $forceDefaults = false) |
||
| 317 | |||
| 318 | /** |
||
| 319 | * Remove extensions from the DB that have been removed from the filesystem. |
||
| 320 | * |
||
| 321 | * @param array $extensionsFromFile |
||
| 322 | * @param array $extensionsFromDB |
||
| 323 | */ |
||
| 324 | private function syncLostExtensions(array $extensionsFromFile, array &$extensionsFromDB) |
||
| 355 | |||
| 356 | /** |
||
| 357 | * Add extensions to the DB that have been added to the filesystem. |
||
| 358 | * - add uninitialized extensions |
||
| 359 | * - update missing or invalid extensions |
||
| 360 | * |
||
| 361 | * @param array $extensionsFromFile |
||
| 362 | * @param array $extensionsFromDB |
||
| 363 | * @return array $upgradedExtensions[<name>] => <version> |
||
| 364 | */ |
||
| 365 | private function syncAddedExtensions(array $extensionsFromFile, array $extensionsFromDB) |
||
| 432 | |||
| 433 | /** |
||
| 434 | * Determine if $min and $max values are compatible with Current Core version |
||
| 435 | * |
||
| 436 | * @param string $compatibilityString Semver |
||
| 437 | * @return bool |
||
| 438 | */ |
||
| 439 | private function isCoreCompatible($compatibilityString) |
||
| 446 | |||
| 447 | /** |
||
| 448 | * Format a compatibility string suitable for semver comparison using vierbergenlars/php-semver |
||
| 449 | * |
||
| 450 | * @param null $coreMin |
||
| 451 | * @param null $coreMax |
||
| 452 | * @return string |
||
| 453 | */ |
||
| 454 | private function formatCoreCompatibilityString($coreMin = null, $coreMax = null) |
||
| 461 | } |
||
| 462 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.