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 Manager 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 Manager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 38 | class Manager implements IManager { |
||
| 39 | const TABLE_ADMIN_SETTINGS = 'admin_settings'; |
||
| 40 | const TABLE_ADMIN_SECTIONS = 'admin_sections'; |
||
| 41 | |||
| 42 | /** @var ILogger */ |
||
| 43 | private $log; |
||
| 44 | /** @var IDBConnection */ |
||
| 45 | private $dbc; |
||
| 46 | /** @var IL10N */ |
||
| 47 | private $l; |
||
| 48 | /** @var IConfig */ |
||
| 49 | private $config; |
||
| 50 | /** @var EncryptionManager */ |
||
| 51 | private $encryptionManager; |
||
| 52 | /** @var IUserManager */ |
||
| 53 | private $userManager; |
||
| 54 | /** @var ILockingProvider */ |
||
| 55 | private $lockingProvider; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @param ILogger $log |
||
| 59 | * @param IDBConnection $dbc |
||
| 60 | * @param IL10N $l |
||
| 61 | * @param IConfig $config |
||
| 62 | * @param EncryptionManager $encryptionManager |
||
| 63 | * @param IUserManager $userManager |
||
| 64 | * @param ILockingProvider $lockingProvider |
||
| 65 | */ |
||
| 66 | View Code Duplication | public function __construct( |
|
|
|
|||
| 67 | ILogger $log, |
||
| 68 | IDBConnection $dbc, |
||
| 69 | IL10N $l, |
||
| 70 | IConfig $config, |
||
| 71 | EncryptionManager $encryptionManager, |
||
| 72 | IUserManager $userManager, |
||
| 73 | ILockingProvider $lockingProvider |
||
| 74 | ) { |
||
| 75 | $this->log = $log; |
||
| 76 | $this->dbc = $dbc; |
||
| 77 | $this->l = $l; |
||
| 78 | $this->config = $config; |
||
| 79 | $this->encryptionManager = $encryptionManager; |
||
| 80 | $this->userManager = $userManager; |
||
| 81 | $this->lockingProvider = $lockingProvider; |
||
| 82 | } |
||
| 83 | |||
| 84 | /** |
||
| 85 | * @inheritdoc |
||
| 86 | */ |
||
| 87 | public function setupSettings(array $settings) { |
||
| 95 | |||
| 96 | /** |
||
| 97 | * attempts to remove an apps section and/or settings entry. A listener is |
||
| 98 | * added centrally making sure that this method is called ones an app was |
||
| 99 | * disabled. |
||
| 100 | * |
||
| 101 | * @param string $appId |
||
| 102 | * @since 9.1.0 |
||
| 103 | */ |
||
| 104 | public function onAppDisabled($appId) { |
||
| 105 | $appInfo = \OC_App::getAppInfo($appId); // hello static legacy |
||
| 106 | |||
| 107 | View Code Duplication | if(isset($appInfo['settings'][IManager::KEY_ADMIN_SECTION])) { |
|
| 108 | $this->remove(self::TABLE_ADMIN_SECTIONS, trim($appInfo['settings'][IManager::KEY_ADMIN_SECTION], '\\')); |
||
| 109 | } |
||
| 110 | View Code Duplication | if(isset($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS])) { |
|
| 111 | $this->remove(self::TABLE_ADMIN_SETTINGS, trim($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS], '\\')); |
||
| 112 | } |
||
| 113 | } |
||
| 114 | |||
| 115 | public function checkForOrphanedClassNames() { |
||
| 116 | $tables = [ self::TABLE_ADMIN_SECTIONS, self::TABLE_ADMIN_SETTINGS ]; |
||
| 117 | foreach ($tables as $table) { |
||
| 118 | $classes = $this->getClasses($table); |
||
| 119 | foreach($classes as $className) { |
||
| 120 | try { |
||
| 121 | \OC::$server->query($className); |
||
| 122 | } catch (QueryException $e) { |
||
| 123 | $this->remove($table, $className); |
||
| 124 | } |
||
| 125 | } |
||
| 126 | } |
||
| 127 | } |
||
| 128 | |||
| 129 | /** |
||
| 130 | * returns the registerd classes in the given table |
||
| 131 | * |
||
| 132 | * @param $table |
||
| 133 | * @return string[] |
||
| 134 | */ |
||
| 135 | private function getClasses($table) { |
||
| 136 | $q = $this->dbc->getQueryBuilder(); |
||
| 137 | $resultStatement = $q->select('class') |
||
| 138 | ->from($table) |
||
| 139 | ->execute(); |
||
| 140 | $data = $resultStatement->fetchAll(); |
||
| 141 | $resultStatement->closeCursor(); |
||
| 142 | |||
| 143 | return array_map(function($row) { return $row['class']; }, $data); |
||
| 144 | } |
||
| 145 | |||
| 146 | /** |
||
| 147 | * @param string $sectionClassName |
||
| 148 | */ |
||
| 149 | View Code Duplication | private function setupAdminSection($sectionClassName) { |
|
| 150 | if(!class_exists($sectionClassName)) { |
||
| 151 | $this->log->debug('Could not find admin section class ' . $sectionClassName); |
||
| 152 | return; |
||
| 153 | } |
||
| 154 | try { |
||
| 155 | $section = $this->query($sectionClassName); |
||
| 156 | } catch (QueryException $e) { |
||
| 157 | // cancel |
||
| 158 | return; |
||
| 159 | } |
||
| 160 | |||
| 161 | if(!$section instanceof ISection) { |
||
| 162 | $this->log->error( |
||
| 163 | 'Admin section instance must implement \OCP\ISection. Invalid class: {class}', |
||
| 164 | ['class' => $sectionClassName] |
||
| 165 | ); |
||
| 166 | return; |
||
| 167 | } |
||
| 168 | if(!$this->hasAdminSection(get_class($section))) { |
||
| 169 | $this->addAdminSection($section); |
||
| 170 | } else { |
||
| 171 | $this->updateAdminSection($section); |
||
| 172 | } |
||
| 173 | } |
||
| 174 | |||
| 175 | View Code Duplication | private function addAdminSection(ISection $section) { |
|
| 182 | |||
| 183 | View Code Duplication | private function addAdminSettings(ISettings $settings) { |
|
| 184 | $this->add(self::TABLE_ADMIN_SETTINGS, [ |
||
| 185 | 'class' => get_class($settings), |
||
| 186 | 'section' => $settings->getSection(), |
||
| 187 | 'priority' => $settings->getPriority(), |
||
| 188 | ]); |
||
| 189 | } |
||
| 190 | |||
| 191 | /** |
||
| 192 | * @param string $table |
||
| 193 | * @param array $values |
||
| 194 | */ |
||
| 195 | private function add($table, array $values) { |
||
| 196 | $query = $this->dbc->getQueryBuilder(); |
||
| 197 | $values = array_map(function($value) use ($query) { |
||
| 198 | return $query->createNamedParameter($value); |
||
| 199 | }, $values); |
||
| 200 | $query->insert($table)->values($values); |
||
| 201 | $query->execute(); |
||
| 202 | } |
||
| 203 | |||
| 204 | View Code Duplication | private function updateAdminSettings(ISettings $settings) { |
|
| 205 | $this->update( |
||
| 206 | self::TABLE_ADMIN_SETTINGS, |
||
| 207 | 'class', |
||
| 208 | get_class($settings), |
||
| 209 | [ |
||
| 210 | 'section' => $settings->getSection(), |
||
| 211 | 'priority' => $settings->getPriority(), |
||
| 212 | ] |
||
| 213 | ); |
||
| 214 | } |
||
| 215 | |||
| 216 | View Code Duplication | private function updateAdminSection(ISection $section) { |
|
| 217 | $this->update( |
||
| 218 | self::TABLE_ADMIN_SECTIONS, |
||
| 219 | 'class', |
||
| 220 | get_class($section), |
||
| 221 | [ |
||
| 222 | 'id' => $section->getID(), |
||
| 223 | 'priority' => $section->getPriority(), |
||
| 224 | ] |
||
| 225 | ); |
||
| 226 | } |
||
| 227 | |||
| 228 | private function update($table, $idCol, $id, $values) { |
||
| 229 | $query = $this->dbc->getQueryBuilder(); |
||
| 230 | $query->update($table); |
||
| 231 | foreach($values as $key => $value) { |
||
| 232 | $query->set($key, $query->createNamedParameter($value)); |
||
| 233 | } |
||
| 234 | $query |
||
| 235 | ->where($query->expr()->eq($idCol, $query->createParameter($idCol))) |
||
| 236 | ->setParameter($idCol, $id) |
||
| 237 | ->execute(); |
||
| 238 | } |
||
| 239 | |||
| 240 | /** |
||
| 241 | * @param string $className |
||
| 242 | * @return bool |
||
| 243 | */ |
||
| 244 | private function hasAdminSection($className) { |
||
| 245 | return $this->has(self::TABLE_ADMIN_SECTIONS, $className); |
||
| 246 | } |
||
| 247 | |||
| 248 | /** |
||
| 249 | * @param string $className |
||
| 250 | * @return bool |
||
| 251 | */ |
||
| 252 | private function hasAdminSettings($className) { |
||
| 253 | return $this->has(self::TABLE_ADMIN_SETTINGS, $className); |
||
| 254 | } |
||
| 255 | |||
| 256 | /** |
||
| 257 | * @param string $table |
||
| 258 | * @param string $className |
||
| 259 | * @return bool |
||
| 260 | */ |
||
| 261 | View Code Duplication | private function has($table, $className) { |
|
| 262 | $query = $this->dbc->getQueryBuilder(); |
||
| 263 | $query->select('class') |
||
| 264 | ->from($table) |
||
| 265 | ->where($query->expr()->eq('class', $query->createNamedParameter($className))) |
||
| 266 | ->setMaxResults(1); |
||
| 267 | |||
| 268 | $result = $query->execute(); |
||
| 269 | $row = $result->fetch(); |
||
| 270 | $result->closeCursor(); |
||
| 271 | |||
| 272 | return (bool) $row; |
||
| 273 | } |
||
| 274 | |||
| 275 | /** |
||
| 276 | * deletes an settings or admin entry from the given table |
||
| 277 | * |
||
| 278 | * @param $table |
||
| 279 | * @param $className |
||
| 280 | */ |
||
| 281 | View Code Duplication | private function remove($table, $className) { |
|
| 282 | $query = $this->dbc->getQueryBuilder(); |
||
| 283 | $query->delete($table) |
||
| 284 | ->where($query->expr()->eq('class', $query->createNamedParameter($className))); |
||
| 285 | |||
| 286 | $query->execute(); |
||
| 287 | } |
||
| 288 | |||
| 289 | View Code Duplication | private function setupAdminSettings($settingsClassName) { |
|
| 290 | if(!class_exists($settingsClassName)) { |
||
| 291 | $this->log->debug('Could not find admin section class ' . $settingsClassName); |
||
| 292 | return; |
||
| 293 | } |
||
| 294 | |||
| 295 | try { |
||
| 296 | /** @var ISettings $settings */ |
||
| 297 | $settings = $this->query($settingsClassName); |
||
| 298 | } catch (QueryException $e) { |
||
| 299 | // cancel |
||
| 300 | return; |
||
| 301 | } |
||
| 302 | |||
| 303 | if(!$settings instanceof ISettings) { |
||
| 304 | $this->log->error( |
||
| 305 | 'Admin section instance must implement \OCP\Settings\ISection. Invalid class: {class}', |
||
| 306 | ['class' => $settingsClassName] |
||
| 307 | ); |
||
| 308 | return; |
||
| 309 | } |
||
| 310 | if(!$this->hasAdminSettings(get_class($settings))) { |
||
| 311 | $this->addAdminSettings($settings); |
||
| 312 | } else { |
||
| 313 | $this->updateAdminSettings($settings); |
||
| 314 | } |
||
| 315 | } |
||
| 316 | |||
| 317 | private function query($className) { |
||
| 318 | try { |
||
| 319 | return \OC::$server->query($className); |
||
| 320 | } catch (QueryException $e) { |
||
| 321 | $this->log->logException($e); |
||
| 322 | throw $e; |
||
| 323 | } |
||
| 324 | } |
||
| 325 | |||
| 326 | /** |
||
| 327 | * @inheritdoc |
||
| 328 | */ |
||
| 329 | public function getAdminSections() { |
||
| 330 | // built-in sections |
||
| 331 | $sections = [ |
||
| 332 | 0 => [new Section('server', $this->l->t('Server settings'), 0)], |
||
| 333 | 5 => [new Section('sharing', $this->l->t('Sharing'), 0)], |
||
| 334 | 45 => [new Section('encryption', $this->l->t('Encryption'), 0)], |
||
| 335 | 98 => [new Section('additional', $this->l->t('Additional settings'), 0)], |
||
| 336 | 99 => [new Section('tips-tricks', $this->l->t('Tips & tricks'), 0)], |
||
| 337 | ]; |
||
| 338 | |||
| 339 | $query = $this->dbc->getQueryBuilder(); |
||
| 340 | $query->selectDistinct('s.class') |
||
| 341 | ->addSelect('s.priority') |
||
| 342 | ->from(self::TABLE_ADMIN_SECTIONS, 's') |
||
| 343 | ->from(self::TABLE_ADMIN_SETTINGS, 'f') |
||
| 344 | ->where($query->expr()->eq('s.id', 'f.section')) |
||
| 345 | ; |
||
| 346 | $result = $query->execute(); |
||
| 347 | |||
| 348 | View Code Duplication | while($row = $result->fetch()) { |
|
| 349 | if(!isset($sections[$row['priority']])) { |
||
| 350 | $sections[$row['priority']] = []; |
||
| 351 | } |
||
| 352 | try { |
||
| 353 | $sections[$row['priority']][] = $this->query($row['class']); |
||
| 354 | } catch (QueryException $e) { |
||
| 355 | // skip |
||
| 356 | } |
||
| 357 | } |
||
| 358 | $result->closeCursor(); |
||
| 359 | |||
| 360 | ksort($sections); |
||
| 361 | return $sections; |
||
| 362 | } |
||
| 363 | |||
| 364 | private function getBuiltInAdminSettings($section) { |
||
| 365 | $forms = []; |
||
| 366 | try { |
||
| 367 | if($section === 'server') { |
||
| 368 | /** @var ISettings $form */ |
||
| 369 | $form = new Admin\Server($this->dbc, $this->config, $this->lockingProvider, $this->l); |
||
| 370 | $forms[$form->getPriority()] = [$form]; |
||
| 371 | $form = new Admin\ServerDevNotice(); |
||
| 372 | $forms[$form->getPriority()] = [$form]; |
||
| 373 | } |
||
| 374 | if($section === 'encryption') { |
||
| 375 | /** @var ISettings $form */ |
||
| 376 | $form = new Admin\Encryption($this->encryptionManager, $this->userManager); |
||
| 377 | $forms[$form->getPriority()] = [$form]; |
||
| 378 | } |
||
| 379 | if($section === 'sharing') { |
||
| 380 | /** @var ISettings $form */ |
||
| 381 | $form = new Admin\Sharing($this->config); |
||
| 382 | $forms[$form->getPriority()] = [$form]; |
||
| 383 | } |
||
| 384 | if($section === 'additional') { |
||
| 385 | /** @var ISettings $form */ |
||
| 386 | $form = new Admin\Additional($this->config); |
||
| 387 | $forms[$form->getPriority()] = [$form]; |
||
| 388 | } |
||
| 389 | if($section === 'tips-tricks') { |
||
| 390 | /** @var ISettings $form */ |
||
| 391 | $form = new Admin\TipsTricks($this->config); |
||
| 392 | $forms[$form->getPriority()] = [$form]; |
||
| 393 | } |
||
| 394 | } catch (QueryException $e) { |
||
| 395 | // skip |
||
| 396 | } |
||
| 397 | return $forms; |
||
| 398 | } |
||
| 399 | |||
| 400 | private function getAdminSettingsFromDB($section, &$settings) { |
||
| 401 | $query = $this->dbc->getQueryBuilder(); |
||
| 402 | $query->select(['class', 'priority']) |
||
| 403 | ->from(self::TABLE_ADMIN_SETTINGS) |
||
| 404 | ->where($query->expr()->eq('section', $this->dbc->getQueryBuilder()->createParameter('section'))) |
||
| 405 | ->setParameter('section', $section); |
||
| 406 | |||
| 407 | $result = $query->execute(); |
||
| 408 | View Code Duplication | while($row = $result->fetch()) { |
|
| 409 | if(!isset($settings[$row['priority']])) { |
||
| 410 | $settings[$row['priority']] = []; |
||
| 411 | } |
||
| 412 | try { |
||
| 413 | $settings[$row['priority']][] = $this->query($row['class']); |
||
| 414 | } catch (QueryException $e) { |
||
| 415 | // skip |
||
| 416 | } |
||
| 417 | } |
||
| 418 | $result->closeCursor(); |
||
| 419 | |||
| 420 | ksort($settings); |
||
| 421 | } |
||
| 422 | |||
| 423 | /** |
||
| 424 | * @inheritdoc |
||
| 425 | */ |
||
| 426 | public function getAdminSettings($section) { |
||
| 431 | } |
||
| 432 |
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.