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 |
||
| 40 | class Manager implements IManager { |
||
| 41 | /** @var IRequest */ |
||
| 42 | protected $request; |
||
| 43 | |||
| 44 | /** @var IUserSession */ |
||
| 45 | protected $session; |
||
| 46 | |||
| 47 | /** @var IConfig */ |
||
| 48 | protected $config; |
||
| 49 | |||
| 50 | /** @var IValidator */ |
||
| 51 | protected $validator; |
||
| 52 | |||
| 53 | /** @var string */ |
||
| 54 | protected $formattingObjectType; |
||
| 55 | |||
| 56 | /** @var int */ |
||
| 57 | protected $formattingObjectId; |
||
| 58 | |||
| 59 | /** @var string */ |
||
| 60 | protected $currentUserId; |
||
| 61 | |||
| 62 | /** |
||
| 63 | * constructor of the controller |
||
| 64 | * |
||
| 65 | * @param IRequest $request |
||
| 66 | * @param IUserSession $session |
||
| 67 | * @param IConfig $config |
||
| 68 | * @param IValidator $validator |
||
| 69 | */ |
||
| 70 | public function __construct(IRequest $request, |
||
| 71 | IUserSession $session, |
||
| 72 | IConfig $config, |
||
| 73 | IValidator $validator) { |
||
| 74 | $this->request = $request; |
||
| 75 | $this->session = $session; |
||
| 76 | $this->config = $config; |
||
| 77 | $this->validator = $validator; |
||
| 78 | } |
||
| 79 | |||
| 80 | /** @var \Closure[] */ |
||
| 81 | private $consumersClosures = array(); |
||
| 82 | |||
| 83 | /** @var IConsumer[] */ |
||
| 84 | private $consumers = array(); |
||
| 85 | |||
| 86 | /** @var \Closure[] */ |
||
| 87 | private $extensionsClosures = array(); |
||
| 88 | |||
| 89 | /** @var IExtension[] */ |
||
| 90 | private $extensions = array(); |
||
| 91 | |||
| 92 | /** @var array list of filters "name" => "is valid" */ |
||
| 93 | protected $validFilters = array( |
||
| 94 | 'all' => true, |
||
| 95 | 'by' => true, |
||
| 96 | 'self' => true, |
||
| 97 | ); |
||
| 98 | |||
| 99 | /** @var array list of type icons "type" => "css class" */ |
||
| 100 | protected $typeIcons = array(); |
||
| 101 | |||
| 102 | /** @var array list of special parameters "app" => ["text" => ["parameter" => "type"]] */ |
||
| 103 | protected $specialParameters = array(); |
||
| 104 | |||
| 105 | /** |
||
| 106 | * @return \OCP\Activity\IConsumer[] |
||
| 107 | */ |
||
| 108 | View Code Duplication | protected function getConsumers() { |
|
|
|
|||
| 109 | if (!empty($this->consumers)) { |
||
| 110 | return $this->consumers; |
||
| 111 | } |
||
| 112 | |||
| 113 | $this->consumers = []; |
||
| 114 | foreach($this->consumersClosures as $consumer) { |
||
| 115 | $c = $consumer(); |
||
| 116 | if ($c instanceof IConsumer) { |
||
| 117 | $this->consumers[] = $c; |
||
| 118 | } else { |
||
| 119 | throw new \InvalidArgumentException('The given consumer does not implement the \OCP\Activity\IConsumer interface'); |
||
| 120 | } |
||
| 121 | } |
||
| 122 | |||
| 123 | return $this->consumers; |
||
| 124 | } |
||
| 125 | |||
| 126 | /** |
||
| 127 | * @return \OCP\Activity\IExtension[] |
||
| 128 | */ |
||
| 129 | View Code Duplication | protected function getExtensions() { |
|
| 130 | if (!empty($this->extensions)) { |
||
| 131 | return $this->extensions; |
||
| 132 | } |
||
| 133 | |||
| 134 | $this->extensions = []; |
||
| 135 | foreach($this->extensionsClosures as $extension) { |
||
| 136 | $e = $extension(); |
||
| 137 | if ($e instanceof IExtension) { |
||
| 138 | $this->extensions[] = $e; |
||
| 139 | } else { |
||
| 140 | throw new \InvalidArgumentException('The given extension does not implement the \OCP\Activity\IExtension interface'); |
||
| 141 | } |
||
| 142 | } |
||
| 143 | |||
| 144 | return $this->extensions; |
||
| 145 | } |
||
| 146 | |||
| 147 | /** |
||
| 148 | * Generates a new IEvent object |
||
| 149 | * |
||
| 150 | * Make sure to call at least the following methods before sending it to the |
||
| 151 | * app with via the publish() method: |
||
| 152 | * - setApp() |
||
| 153 | * - setType() |
||
| 154 | * - setAffectedUser() |
||
| 155 | * - setSubject() |
||
| 156 | * |
||
| 157 | * @return IEvent |
||
| 158 | */ |
||
| 159 | public function generateEvent() { |
||
| 160 | return new Event($this->validator); |
||
| 161 | } |
||
| 162 | |||
| 163 | /** |
||
| 164 | * Publish an event to the activity consumers |
||
| 165 | * |
||
| 166 | * Make sure to call at least the following methods before sending an Event: |
||
| 167 | * - setApp() |
||
| 168 | * - setType() |
||
| 169 | * - setAffectedUser() |
||
| 170 | * - setSubject() |
||
| 171 | * |
||
| 172 | * @param IEvent $event |
||
| 173 | * @throws \BadMethodCallException if required values have not been set |
||
| 174 | */ |
||
| 175 | public function publish(IEvent $event) { |
||
| 176 | if ($event->getAuthor() === '') { |
||
| 177 | if ($this->session->getUser() instanceof IUser) { |
||
| 178 | $event->setAuthor($this->session->getUser()->getUID()); |
||
| 179 | } |
||
| 180 | } |
||
| 181 | |||
| 182 | if (!$event->getTimestamp()) { |
||
| 183 | $event->setTimestamp(time()); |
||
| 184 | } |
||
| 185 | |||
| 186 | if (!$event->isValid()) { |
||
| 187 | throw new \BadMethodCallException('The given event is invalid'); |
||
| 188 | } |
||
| 189 | |||
| 190 | foreach ($this->getConsumers() as $c) { |
||
| 191 | $c->receive($event); |
||
| 192 | } |
||
| 193 | } |
||
| 194 | |||
| 195 | /** |
||
| 196 | * @param string $app The app where this event is associated with |
||
| 197 | * @param string $subject A short description of the event |
||
| 198 | * @param array $subjectParams Array with parameters that are filled in the subject |
||
| 199 | * @param string $message A longer description of the event |
||
| 200 | * @param array $messageParams Array with parameters that are filled in the message |
||
| 201 | * @param string $file The file including path where this event is associated with |
||
| 202 | * @param string $link A link where this event is associated with |
||
| 203 | * @param string $affectedUser Recipient of the activity |
||
| 204 | * @param string $type Type of the notification |
||
| 205 | * @param int $priority Priority of the notification |
||
| 206 | */ |
||
| 207 | public function publishActivity($app, $subject, $subjectParams, $message, $messageParams, $file, $link, $affectedUser, $type, $priority) { |
||
| 208 | $event = $this->generateEvent(); |
||
| 209 | $event->setApp($app) |
||
| 210 | ->setType($type) |
||
| 211 | ->setAffectedUser($affectedUser) |
||
| 212 | ->setSubject($subject, $subjectParams) |
||
| 213 | ->setMessage($message, $messageParams) |
||
| 214 | ->setObject('', 0, $file) |
||
| 215 | ->setLink($link); |
||
| 216 | |||
| 217 | $this->publish($event); |
||
| 218 | } |
||
| 219 | |||
| 220 | /** |
||
| 221 | * In order to improve lazy loading a closure can be registered which will be called in case |
||
| 222 | * activity consumers are actually requested |
||
| 223 | * |
||
| 224 | * $callable has to return an instance of OCA\Activity\IConsumer |
||
| 225 | * |
||
| 226 | * @param \Closure $callable |
||
| 227 | */ |
||
| 228 | public function registerConsumer(\Closure $callable) { |
||
| 232 | |||
| 233 | /** |
||
| 234 | * In order to improve lazy loading a closure can be registered which will be called in case |
||
| 235 | * activity consumers are actually requested |
||
| 236 | * |
||
| 237 | * $callable has to return an instance of OCA\Activity\IExtension |
||
| 238 | * |
||
| 239 | * @param \Closure $callable |
||
| 240 | */ |
||
| 241 | public function registerExtension(\Closure $callable) { |
||
| 245 | |||
| 246 | /** @var string[] */ |
||
| 247 | protected $filterClasses = []; |
||
| 248 | |||
| 249 | /** @var IFilter[] */ |
||
| 250 | protected $filters = []; |
||
| 251 | |||
| 252 | /** @var bool */ |
||
| 253 | protected $loadedLegacyFilters = false; |
||
| 254 | |||
| 255 | /** |
||
| 256 | * @param string $filter Class must implement OCA\Activity\IFilter |
||
| 257 | * @return void |
||
| 258 | */ |
||
| 259 | public function registerFilter($filter) { |
||
| 262 | |||
| 263 | /** |
||
| 264 | * @return IFilter[] |
||
| 265 | * @throws \InvalidArgumentException |
||
| 266 | */ |
||
| 267 | public function getFilters() { |
||
| 299 | |||
| 300 | /** |
||
| 301 | * @param string $id |
||
| 302 | * @return IFilter |
||
| 303 | * @throws \InvalidArgumentException when the filter was not found |
||
| 304 | * @since 11.0.0 |
||
| 305 | */ |
||
| 306 | public function getFilterById($id) { |
||
| 315 | |||
| 316 | /** @var string[] */ |
||
| 317 | protected $providerClasses = []; |
||
| 318 | |||
| 319 | /** @var IProvider[] */ |
||
| 320 | protected $providers = []; |
||
| 321 | |||
| 322 | /** |
||
| 323 | * @param string $provider Class must implement OCA\Activity\IProvider |
||
| 324 | * @return void |
||
| 325 | */ |
||
| 326 | public function registerProvider($provider) { |
||
| 329 | |||
| 330 | /** |
||
| 331 | * @return IProvider[] |
||
| 332 | * @throws \InvalidArgumentException |
||
| 333 | */ |
||
| 334 | public function getProviders() { |
||
| 349 | |||
| 350 | /** @var string[] */ |
||
| 351 | protected $settingsClasses = []; |
||
| 352 | |||
| 353 | /** @var ISetting[] */ |
||
| 354 | protected $settings = []; |
||
| 355 | |||
| 356 | /** @var bool */ |
||
| 357 | protected $loadedLegacyTypes = false; |
||
| 358 | |||
| 359 | /** |
||
| 360 | * @param string $setting Class must implement OCA\Activity\ISetting |
||
| 361 | * @return void |
||
| 362 | */ |
||
| 363 | public function registerSetting($setting) { |
||
| 366 | |||
| 367 | /** |
||
| 368 | * @return ISetting[] |
||
| 369 | * @throws \InvalidArgumentException |
||
| 370 | */ |
||
| 371 | public function getSettings() { |
||
| 411 | |||
| 412 | /** |
||
| 413 | * @param string $id |
||
| 414 | * @return ISetting |
||
| 415 | * @throws \InvalidArgumentException when the setting was not found |
||
| 416 | * @since 11.0.0 |
||
| 417 | */ |
||
| 418 | public function getSettingById($id) { |
||
| 427 | |||
| 428 | /** |
||
| 429 | * @param string $type |
||
| 430 | * @return string |
||
| 431 | */ |
||
| 432 | public function getTypeIcon($type) { |
||
| 448 | |||
| 449 | /** |
||
| 450 | * @param string $type |
||
| 451 | * @param string $id |
||
| 452 | */ |
||
| 453 | public function setFormattingObject($type, $id) { |
||
| 457 | |||
| 458 | /** |
||
| 459 | * @return bool |
||
| 460 | */ |
||
| 461 | public function isFormattingFilteredObject() { |
||
| 466 | |||
| 467 | /** |
||
| 468 | * @param string $app |
||
| 469 | * @param string $text |
||
| 470 | * @param array $params |
||
| 471 | * @param boolean $stripPath |
||
| 472 | * @param boolean $highlightParams |
||
| 473 | * @param string $languageCode |
||
| 474 | * @return string|false |
||
| 475 | */ |
||
| 476 | public function translate($app, $text, $params, $stripPath, $highlightParams, $languageCode) { |
||
| 486 | |||
| 487 | /** |
||
| 488 | * @param string $app |
||
| 489 | * @param string $text |
||
| 490 | * @return array|false |
||
| 491 | */ |
||
| 492 | public function getSpecialParameterList($app, $text) { |
||
| 512 | |||
| 513 | /** |
||
| 514 | * @param array $activity |
||
| 515 | * @return integer|false |
||
| 516 | */ |
||
| 517 | public function getGroupParameter($activity) { |
||
| 527 | |||
| 528 | /** |
||
| 529 | * Set the user we need to use |
||
| 530 | * |
||
| 531 | * @param string|null $currentUserId |
||
| 532 | * @throws \UnexpectedValueException If the user is invalid |
||
| 533 | */ |
||
| 534 | public function setCurrentUserId($currentUserId) { |
||
| 540 | |||
| 541 | /** |
||
| 542 | * Get the user we need to use |
||
| 543 | * |
||
| 544 | * Either the user is logged in, or we try to get it from the token |
||
| 545 | * |
||
| 546 | * @return string |
||
| 547 | * @throws \UnexpectedValueException If the token is invalid, does not exist or is not unique |
||
| 548 | */ |
||
| 549 | public function getCurrentUserId() { |
||
| 558 | |||
| 559 | /** |
||
| 560 | * Get the user for the token |
||
| 561 | * |
||
| 562 | * @return string |
||
| 563 | * @throws \UnexpectedValueException If the token is invalid, does not exist or is not unique |
||
| 564 | */ |
||
| 565 | protected function getUserFromToken() { |
||
| 581 | |||
| 582 | /** |
||
| 583 | * @return array |
||
| 584 | * @deprecated 11.0.0 - Use getFilters() instead |
||
| 585 | */ |
||
| 586 | public function getNavigation() { |
||
| 601 | |||
| 602 | /** |
||
| 603 | * @param string $filterValue |
||
| 604 | * @return boolean |
||
| 605 | * @deprecated 11.0.0 - Use getFilterById() instead |
||
| 606 | */ |
||
| 607 | public function isFilterValid($filterValue) { |
||
| 622 | |||
| 623 | /** |
||
| 624 | * @param array $types |
||
| 625 | * @param string $filter |
||
| 626 | * @return array |
||
| 627 | * @deprecated 11.0.0 - Use getFilterById()->filterTypes() instead |
||
| 628 | */ |
||
| 629 | public function filterNotificationTypes($types, $filter) { |
||
| 642 | |||
| 643 | /** |
||
| 644 | * @param string $filter |
||
| 645 | * @return array |
||
| 646 | * @deprecated 11.0.0 - Use getFilterById() instead |
||
| 647 | */ |
||
| 648 | public function getQueryForFilter($filter) { |
||
| 673 | |||
| 674 | /** |
||
| 675 | * Will return additional notification types as specified by other apps |
||
| 676 | * |
||
| 677 | * @param string $languageCode |
||
| 678 | * @return array |
||
| 679 | * @deprecated 11.0.0 - Use getSettings() instead |
||
| 680 | */ |
||
| 681 | public function getNotificationTypes($languageCode) { |
||
| 697 | |||
| 698 | /** |
||
| 699 | * @param string $method |
||
| 700 | * @return array |
||
| 701 | * @deprecated 11.0.0 - Use getSettings()->isDefaulEnabled<method>() instead |
||
| 702 | */ |
||
| 703 | public function getDefaultTypes($method) { |
||
| 713 | } |
||
| 714 |
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.