nextcloud /
server
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 |
||
| 42 | class Manager implements IManager { |
||
| 43 | /** @var IRequest */ |
||
| 44 | protected $request; |
||
| 45 | |||
| 46 | /** @var IUserSession */ |
||
| 47 | protected $session; |
||
| 48 | |||
| 49 | /** @var IConfig */ |
||
| 50 | protected $config; |
||
| 51 | |||
| 52 | /** @var IValidator */ |
||
| 53 | protected $validator; |
||
| 54 | |||
| 55 | /** @var string */ |
||
| 56 | protected $formattingObjectType; |
||
| 57 | |||
| 58 | /** @var int */ |
||
| 59 | protected $formattingObjectId; |
||
| 60 | |||
| 61 | /** @var bool */ |
||
| 62 | protected $requirePNG; |
||
| 63 | |||
| 64 | /** @var string */ |
||
| 65 | protected $currentUserId; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * constructor of the controller |
||
| 69 | * |
||
| 70 | * @param IRequest $request |
||
| 71 | * @param IUserSession $session |
||
| 72 | * @param IConfig $config |
||
| 73 | * @param IValidator $validator |
||
| 74 | */ |
||
| 75 | public function __construct(IRequest $request, |
||
| 84 | |||
| 85 | /** @var \Closure[] */ |
||
| 86 | private $consumersClosures = array(); |
||
| 87 | |||
| 88 | /** @var IConsumer[] */ |
||
| 89 | private $consumers = array(); |
||
| 90 | |||
| 91 | /** @var \Closure[] */ |
||
| 92 | private $extensionsClosures = array(); |
||
| 93 | |||
| 94 | /** @var IExtension[] */ |
||
| 95 | private $extensions = array(); |
||
| 96 | |||
| 97 | /** @var array list of filters "name" => "is valid" */ |
||
| 98 | protected $validFilters = array( |
||
| 99 | 'all' => true, |
||
| 100 | 'by' => true, |
||
| 101 | 'self' => true, |
||
| 102 | ); |
||
| 103 | |||
| 104 | /** @var array list of type icons "type" => "css class" */ |
||
| 105 | protected $typeIcons = array(); |
||
| 106 | |||
| 107 | /** @var array list of special parameters "app" => ["text" => ["parameter" => "type"]] */ |
||
| 108 | protected $specialParameters = array(); |
||
| 109 | |||
| 110 | /** |
||
| 111 | * @return \OCP\Activity\IConsumer[] |
||
| 112 | */ |
||
| 113 | View Code Duplication | protected function getConsumers() { |
|
| 130 | |||
| 131 | /** |
||
| 132 | * @return \OCP\Activity\IExtension[] |
||
| 133 | */ |
||
| 134 | View Code Duplication | protected function getExtensions() { |
|
| 151 | |||
| 152 | /** |
||
| 153 | * Generates a new IEvent object |
||
| 154 | * |
||
| 155 | * Make sure to call at least the following methods before sending it to the |
||
| 156 | * app with via the publish() method: |
||
| 157 | * - setApp() |
||
| 158 | * - setType() |
||
| 159 | * - setAffectedUser() |
||
| 160 | * - setSubject() |
||
| 161 | * |
||
| 162 | * @return IEvent |
||
| 163 | */ |
||
| 164 | public function generateEvent() { |
||
| 167 | |||
| 168 | /** |
||
| 169 | * Publish an event to the activity consumers |
||
| 170 | * |
||
| 171 | * Make sure to call at least the following methods before sending an Event: |
||
| 172 | * - setApp() |
||
| 173 | * - setType() |
||
| 174 | * - setAffectedUser() |
||
| 175 | * - setSubject() |
||
| 176 | * |
||
| 177 | * @param IEvent $event |
||
| 178 | * @throws \BadMethodCallException if required values have not been set |
||
| 179 | */ |
||
| 180 | public function publish(IEvent $event) { |
||
| 199 | |||
| 200 | /** |
||
| 201 | * In order to improve lazy loading a closure can be registered which will be called in case |
||
| 202 | * activity consumers are actually requested |
||
| 203 | * |
||
| 204 | * $callable has to return an instance of OCA\Activity\IConsumer |
||
| 205 | * |
||
| 206 | * @param \Closure $callable |
||
| 207 | */ |
||
| 208 | public function registerConsumer(\Closure $callable) { |
||
| 212 | |||
| 213 | /** |
||
| 214 | * In order to improve lazy loading a closure can be registered which will be called in case |
||
| 215 | * activity consumers are actually requested |
||
| 216 | * |
||
| 217 | * $callable has to return an instance of OCA\Activity\IExtension |
||
| 218 | * |
||
| 219 | * @param \Closure $callable |
||
| 220 | */ |
||
| 221 | public function registerExtension(\Closure $callable) { |
||
| 225 | |||
| 226 | /** @var string[] */ |
||
| 227 | protected $filterClasses = []; |
||
| 228 | |||
| 229 | /** @var IFilter[] */ |
||
| 230 | protected $filters = []; |
||
| 231 | |||
| 232 | /** @var bool */ |
||
| 233 | protected $loadedLegacyFilters = false; |
||
| 234 | |||
| 235 | /** |
||
| 236 | * @param string $filter Class must implement OCA\Activity\IFilter |
||
| 237 | * @return void |
||
| 238 | */ |
||
| 239 | public function registerFilter($filter) { |
||
| 242 | |||
| 243 | /** |
||
| 244 | * @return IFilter[] |
||
| 245 | * @throws \InvalidArgumentException |
||
| 246 | */ |
||
| 247 | public function getFilters() { |
||
| 279 | |||
| 280 | /** |
||
| 281 | * @param string $id |
||
| 282 | * @return IFilter |
||
| 283 | * @throws \InvalidArgumentException when the filter was not found |
||
| 284 | * @since 11.0.0 |
||
| 285 | */ |
||
| 286 | public function getFilterById($id) { |
||
| 295 | |||
| 296 | /** @var string[] */ |
||
| 297 | protected $providerClasses = []; |
||
| 298 | |||
| 299 | /** @var IProvider[] */ |
||
| 300 | protected $providers = []; |
||
| 301 | |||
| 302 | /** |
||
| 303 | * @param string $provider Class must implement OCA\Activity\IProvider |
||
| 304 | * @return void |
||
| 305 | */ |
||
| 306 | public function registerProvider($provider) { |
||
| 309 | |||
| 310 | /** |
||
| 311 | * @return IProvider[] |
||
| 312 | * @throws \InvalidArgumentException |
||
| 313 | */ |
||
| 314 | public function getProviders() { |
||
| 329 | |||
| 330 | /** @var string[] */ |
||
| 331 | protected $settingsClasses = []; |
||
| 332 | |||
| 333 | /** @var ISetting[] */ |
||
| 334 | protected $settings = []; |
||
| 335 | |||
| 336 | /** @var bool */ |
||
| 337 | protected $loadedLegacyTypes = false; |
||
| 338 | |||
| 339 | /** |
||
| 340 | * @param string $setting Class must implement OCA\Activity\ISetting |
||
| 341 | * @return void |
||
| 342 | */ |
||
| 343 | public function registerSetting($setting) { |
||
| 346 | |||
| 347 | /** |
||
| 348 | * @return ISetting[] |
||
| 349 | * @throws \InvalidArgumentException |
||
| 350 | */ |
||
| 351 | public function getSettings() { |
||
| 391 | |||
| 392 | /** |
||
| 393 | * @param string $id |
||
| 394 | * @return ISetting |
||
| 395 | * @throws \InvalidArgumentException when the setting was not found |
||
| 396 | * @since 11.0.0 |
||
| 397 | */ |
||
| 398 | public function getSettingById($id) { |
||
| 407 | |||
| 408 | /** |
||
| 409 | * @param string $type |
||
| 410 | * @return string |
||
| 411 | */ |
||
| 412 | public function getTypeIcon($type) { |
||
| 428 | |||
| 429 | /** |
||
| 430 | * @param string $type |
||
| 431 | * @param string $id |
||
| 432 | */ |
||
| 433 | public function setFormattingObject($type, $id) { |
||
| 437 | |||
| 438 | /** |
||
| 439 | * @return bool |
||
| 440 | */ |
||
| 441 | public function isFormattingFilteredObject() { |
||
| 446 | |||
| 447 | /** |
||
| 448 | * @param bool $status Set to true, when parsing events should not use SVG icons |
||
| 449 | */ |
||
| 450 | public function setRequirePNG($status) { |
||
| 453 | |||
| 454 | /** |
||
| 455 | * @return bool |
||
| 456 | */ |
||
| 457 | public function getRequirePNG() { |
||
| 460 | |||
| 461 | /** |
||
| 462 | * @param string $app |
||
| 463 | * @param string $text |
||
| 464 | * @param array $params |
||
| 465 | * @param boolean $stripPath |
||
| 466 | * @param boolean $highlightParams |
||
| 467 | * @param string $languageCode |
||
| 468 | * @return string|false |
||
| 469 | */ |
||
| 470 | public function translate($app, $text, $params, $stripPath, $highlightParams, $languageCode) { |
||
| 480 | |||
| 481 | /** |
||
| 482 | * @param string $app |
||
| 483 | * @param string $text |
||
| 484 | * @return array|false |
||
| 485 | */ |
||
| 486 | public function getSpecialParameterList($app, $text) { |
||
| 506 | |||
| 507 | /** |
||
| 508 | * @param array $activity |
||
| 509 | * @return integer|false |
||
| 510 | */ |
||
| 511 | public function getGroupParameter($activity) { |
||
| 521 | |||
| 522 | /** |
||
| 523 | * Set the user we need to use |
||
| 524 | * |
||
| 525 | * @param string|null $currentUserId |
||
| 526 | * @throws \UnexpectedValueException If the user is invalid |
||
| 527 | */ |
||
| 528 | public function setCurrentUserId($currentUserId) { |
||
| 534 | |||
| 535 | /** |
||
| 536 | * Get the user we need to use |
||
| 537 | * |
||
| 538 | * Either the user is logged in, or we try to get it from the token |
||
| 539 | * |
||
| 540 | * @return string |
||
| 541 | * @throws \UnexpectedValueException If the token is invalid, does not exist or is not unique |
||
| 542 | */ |
||
| 543 | public function getCurrentUserId() { |
||
| 552 | |||
| 553 | /** |
||
| 554 | * Get the user for the token |
||
| 555 | * |
||
| 556 | * @return string |
||
| 557 | * @throws \UnexpectedValueException If the token is invalid, does not exist or is not unique |
||
| 558 | */ |
||
| 559 | protected function getUserFromToken() { |
||
| 575 | |||
| 576 | /** |
||
| 577 | * @return array |
||
| 578 | * @deprecated 11.0.0 - Use getFilters() instead |
||
| 579 | */ |
||
| 580 | public function getNavigation() { |
||
| 595 | |||
| 596 | /** |
||
| 597 | * @param string $filterValue |
||
| 598 | * @return boolean |
||
| 599 | * @deprecated 11.0.0 - Use getFilterById() instead |
||
| 600 | */ |
||
| 601 | public function isFilterValid($filterValue) { |
||
| 616 | |||
| 617 | /** |
||
| 618 | * @param array $types |
||
| 619 | * @param string $filter |
||
| 620 | * @return array |
||
| 621 | * @deprecated 11.0.0 - Use getFilterById()->filterTypes() instead |
||
| 622 | */ |
||
| 623 | public function filterNotificationTypes($types, $filter) { |
||
| 636 | |||
| 637 | /** |
||
| 638 | * @param string $filter |
||
| 639 | * @return array |
||
| 640 | * @deprecated 11.0.0 - Use getFilterById() instead |
||
| 641 | */ |
||
| 642 | public function getQueryForFilter($filter) { |
||
| 667 | |||
| 668 | /** |
||
| 669 | * Will return additional notification types as specified by other apps |
||
| 670 | * |
||
| 671 | * @param string $languageCode |
||
| 672 | * @return array |
||
| 673 | * @deprecated 11.0.0 - Use getSettings() instead |
||
| 674 | */ |
||
| 675 | public function getNotificationTypes($languageCode) { |
||
| 686 | |||
| 687 | /** |
||
| 688 | * @param string $method |
||
| 689 | * @return array |
||
| 690 | * @deprecated 11.0.0 - Use getSettings()->isDefaulEnabled<method>() instead |
||
| 691 | */ |
||
| 692 | public function getDefaultTypes($method) { |
||
| 702 | } |
||
| 703 |
Loading history...
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.