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 ActivityManager 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 ActivityManager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 48 | class ActivityManager { |
||
| 49 | |||
| 50 | private $manager; |
||
| 51 | private $userId; |
||
| 52 | private $permissionService; |
||
| 53 | private $boardMapper; |
||
| 54 | private $cardMapper; |
||
| 55 | private $attachmentMapper; |
||
| 56 | private $aclMapper; |
||
| 57 | private $stackMapper; |
||
| 58 | private $l10n; |
||
| 59 | |||
| 60 | const DECK_OBJECT_BOARD = 'deck_board'; |
||
| 61 | const DECK_OBJECT_CARD = 'deck_card'; |
||
| 62 | |||
| 63 | const SUBJECT_BOARD_CREATE = 'board_create'; |
||
| 64 | const SUBJECT_BOARD_UPDATE = 'board_update'; |
||
| 65 | const SUBJECT_BOARD_UPDATE_TITLE = 'board_update_title'; |
||
| 66 | const SUBJECT_BOARD_UPDATE_ARCHIVED = 'board_update_archived'; |
||
| 67 | const SUBJECT_BOARD_DELETE = 'board_delete'; |
||
| 68 | const SUBJECT_BOARD_RESTORE = 'board_restore'; |
||
| 69 | const SUBJECT_BOARD_SHARE = 'board_share'; |
||
| 70 | const SUBJECT_BOARD_UNSHARE = 'board_unshare'; |
||
| 71 | |||
| 72 | const SUBJECT_STACK_CREATE = 'stack_create'; |
||
| 73 | const SUBJECT_STACK_UPDATE = 'stack_update'; |
||
| 74 | const SUBJECT_STACK_UPDATE_TITLE = 'stack_update_title'; |
||
| 75 | const SUBJECT_STACK_UPDATE_ORDER = 'stack_update_order'; |
||
| 76 | const SUBJECT_STACK_DELETE = 'stack_delete'; |
||
| 77 | |||
| 78 | const SUBJECT_CARD_CREATE = 'card_create'; |
||
| 79 | const SUBJECT_CARD_DELETE = 'card_delete'; |
||
| 80 | const SUBJECT_CARD_RESTORE = 'card_restore'; |
||
| 81 | const SUBJECT_CARD_UPDATE = 'card_update'; |
||
| 82 | const SUBJECT_CARD_UPDATE_TITLE = 'card_update_title'; |
||
| 83 | const SUBJECT_CARD_UPDATE_DESCRIPTION = 'card_update_description'; |
||
| 84 | const SUBJECT_CARD_UPDATE_DUEDATE = 'card_update_duedate'; |
||
| 85 | const SUBJECT_CARD_UPDATE_ARCHIVE = 'card_update_archive'; |
||
| 86 | const SUBJECT_CARD_UPDATE_UNARCHIVE = 'card_update_unarchive'; |
||
| 87 | const SUBJECT_CARD_UPDATE_STACKID = 'card_update_stackId'; |
||
| 88 | const SUBJECT_CARD_USER_ASSIGN = 'card_user_assign'; |
||
| 89 | const SUBJECT_CARD_USER_UNASSIGN = 'card_user_unassign'; |
||
| 90 | |||
| 91 | const SUBJECT_ATTACHMENT_CREATE = 'attachment_create'; |
||
| 92 | const SUBJECT_ATTACHMENT_UPDATE = 'attachment_update'; |
||
| 93 | const SUBJECT_ATTACHMENT_DELETE = 'attachment_delete'; |
||
| 94 | const SUBJECT_ATTACHMENT_RESTORE = 'attachment_restore'; |
||
| 95 | |||
| 96 | const SUBJECT_LABEL_CREATE = 'label_create'; |
||
| 97 | const SUBJECT_LABEL_UPDATE = 'label_update'; |
||
| 98 | const SUBJECT_LABEL_DELETE = 'label_delete'; |
||
| 99 | const SUBJECT_LABEL_ASSIGN = 'label_assign'; |
||
| 100 | const SUBJECT_LABEL_UNASSING = 'label_unassign'; |
||
| 101 | |||
| 102 | const SUBJECT_CARD_COMMENT_CREATE = 'card_comment_create'; |
||
| 103 | |||
| 104 | View Code Duplication | public function __construct( |
|
|
|
|||
| 105 | IManager $manager, |
||
| 106 | PermissionService $permissionsService, |
||
| 107 | BoardMapper $boardMapper, |
||
| 108 | CardMapper $cardMapper, |
||
| 109 | StackMapper $stackMapper, |
||
| 110 | AttachmentMapper $attachmentMapper, |
||
| 111 | AclMapper $aclMapper, |
||
| 112 | IL10N $l10n, |
||
| 113 | $userId |
||
| 114 | ) { |
||
| 115 | $this->manager = $manager; |
||
| 116 | $this->permissionService = $permissionsService; |
||
| 117 | $this->boardMapper = $boardMapper; |
||
| 118 | $this->cardMapper = $cardMapper; |
||
| 119 | $this->stackMapper = $stackMapper; |
||
| 120 | $this->attachmentMapper = $attachmentMapper; |
||
| 121 | $this->aclMapper = $aclMapper; |
||
| 122 | $this->l10n = $l10n; |
||
| 123 | $this->userId = $userId; |
||
| 124 | } |
||
| 125 | |||
| 126 | /** |
||
| 127 | * @param $subjectIdentifier |
||
| 128 | * @param array $subjectParams |
||
| 129 | * @param bool $ownActivity |
||
| 130 | * @return string |
||
| 131 | */ |
||
| 132 | public function getActivityFormat($subjectIdentifier, $subjectParams = [], $ownActivity = false) { |
||
| 133 | $subject = ''; |
||
| 134 | switch ($subjectIdentifier) { |
||
| 135 | View Code Duplication | case self::SUBJECT_BOARD_CREATE: |
|
| 136 | $subject = $ownActivity ? $this->l10n->t('You have created a new board {board}'): $this->l10n->t('{user} has created a new board {board}'); |
||
| 137 | break; |
||
| 138 | View Code Duplication | case self::SUBJECT_BOARD_DELETE: |
|
| 139 | $subject = $ownActivity ? $this->l10n->t('You have deleted the board {board}') : $this->l10n->t('{user} has deleted the board {board}'); |
||
| 140 | break; |
||
| 141 | View Code Duplication | case self::SUBJECT_BOARD_RESTORE: |
|
| 142 | $subject = $ownActivity ? $this->l10n->t('You have restored the board {board}') : $this->l10n->t('{user} has restored the board {board}'); |
||
| 143 | break; |
||
| 144 | View Code Duplication | case self::SUBJECT_BOARD_SHARE: |
|
| 145 | $subject = $ownActivity ? $this->l10n->t('You have shared the board {board} with {acl}') : $this->l10n->t('{user} has shared the board {board} with {sharee}'); |
||
| 146 | break; |
||
| 147 | View Code Duplication | case self::SUBJECT_BOARD_UNSHARE: |
|
| 148 | $subject = $ownActivity ? $this->l10n->t('You have removed {acl} from the board {board}') : $this->l10n->t('{user} has removed {acl} from the board {board}'); |
||
| 149 | break; |
||
| 150 | |||
| 151 | View Code Duplication | case self::SUBJECT_BOARD_UPDATE_TITLE: |
|
| 152 | $subject = $ownActivity ? $this->l10n->t('You have renamed the board {before} to {board}') : $this->l10n->t('{user} has has renamed the board {before} to {board}'); |
||
| 153 | break; |
||
| 154 | View Code Duplication | case self::SUBJECT_BOARD_UPDATE_ARCHIVED: |
|
| 155 | if (isset($subjectParams['after']) && $subjectParams['after']) { |
||
| 156 | $subject = $ownActivity ? $this->l10n->t('You have archived the board {board}') : $this->l10n->t('{user} has archived the board {before}'); |
||
| 157 | } else { |
||
| 158 | $subject = $ownActivity ? $this->l10n->t('You have unarchived the board {board}') : $this->l10n->t('{user} has unarchived the board {before}'); |
||
| 159 | } |
||
| 160 | break; |
||
| 161 | |||
| 162 | View Code Duplication | case self::SUBJECT_STACK_CREATE: |
|
| 163 | $subject = $ownActivity ? $this->l10n->t('You have created a new stack {stack} on {board}') : $this->l10n->t('{user} has created a new stack {stack} on {board}'); |
||
| 164 | break; |
||
| 165 | View Code Duplication | case self::SUBJECT_STACK_UPDATE: |
|
| 166 | $subject = $ownActivity ? $this->l10n->t('You have created a new stack {stack} on {board}') : $this->l10n->t('{user} has created a new stack {stack} on {board}'); |
||
| 167 | break; |
||
| 168 | View Code Duplication | case self::SUBJECT_STACK_UPDATE_TITLE: |
|
| 169 | $subject = $ownActivity ? $this->l10n->t('You have renamed a new stack {before} to {stack} on {board}') : $this->l10n->t('{user} has renamed a new stack {before} to {stack} on {board}'); |
||
| 170 | break; |
||
| 171 | View Code Duplication | case self::SUBJECT_STACK_DELETE: |
|
| 172 | $subject = $ownActivity ? $this->l10n->t('You have deleted {stack} on {board}') : $this->l10n->t('{user} has deleted {stack} on {board}'); |
||
| 173 | break; |
||
| 174 | View Code Duplication | case self::SUBJECT_CARD_CREATE: |
|
| 175 | $subject = $ownActivity ? $this->l10n->t('You have created {card} in {stack} on {board}') : $this->l10n->t('{user} has created {card} in {stack} on {board}'); |
||
| 176 | break; |
||
| 177 | View Code Duplication | case self::SUBJECT_CARD_DELETE: |
|
| 178 | $subject = $ownActivity ? $this->l10n->t('You have deleted {card} in {stack} on {board}') : $this->l10n->t('{user} has deleted {card} in {stack} on {board}'); |
||
| 179 | break; |
||
| 180 | View Code Duplication | case self::SUBJECT_CARD_UPDATE_TITLE: |
|
| 181 | $subject = $ownActivity ? $this->l10n->t('You have renamed the card {before} to {card}') : $this->l10n->t('{user} has renamed the card {before} to {card}'); |
||
| 182 | break; |
||
| 183 | View Code Duplication | case self::SUBJECT_CARD_UPDATE_DESCRIPTION: |
|
| 184 | if (!isset($subjectParams['before'])) { |
||
| 185 | $subject = $ownActivity ? $this->l10n->t('You have added a description to {card} in {stack} on {board}') : $this->l10n->t('{user} has added a description to {card} in {stack} on {board}'); |
||
| 186 | } else { |
||
| 187 | $subject = $ownActivity ? $this->l10n->t('You have updated the description of {card} in {stack} on {board}') : $this->l10n->t('{user} has updated the description {card} in {stack} on {board}'); |
||
| 188 | } |
||
| 189 | break; |
||
| 190 | View Code Duplication | case self::SUBJECT_CARD_UPDATE_ARCHIVE: |
|
| 191 | $subject = $ownActivity ? $this->l10n->t('You have archived {card} in {stack} on {board}') : $this->l10n->t('{user} has archived {card} in {stack} on {board}'); |
||
| 192 | break; |
||
| 193 | View Code Duplication | case self::SUBJECT_CARD_UPDATE_UNARCHIVE: |
|
| 194 | $subject = $ownActivity ? $this->l10n->t('You have unarchived {card} in {stack} on {board}') : $this->l10n->t('{user} has unarchived {card} in {stack} on {board}'); |
||
| 195 | break; |
||
| 196 | case self::SUBJECT_CARD_UPDATE_DUEDATE: |
||
| 197 | if (!isset($subjectParams['after'])) { |
||
| 198 | $subject = $ownActivity ? $this->l10n->t('You have removed the due date of {card}') : $this->l10n->t('{user} has removed the due date of {card}'); |
||
| 199 | } else if (isset($subjectParams['before']) && !isset($subjectParams['after'])) { |
||
| 200 | $subject = $ownActivity ? $this->l10n->t('You have set the due date of {card} to {after}') : $this->l10n->t('{user} has set the due date of {card} to {after}'); |
||
| 201 | } else { |
||
| 202 | $subject = $ownActivity ? $this->l10n->t('You have updated the due date of {card} to {after}') : $this->l10n->t('{user} has updated the due date of {card} to {after}'); |
||
| 203 | } |
||
| 204 | |||
| 205 | break; |
||
| 206 | View Code Duplication | case self::SUBJECT_LABEL_ASSIGN: |
|
| 207 | $subject = $ownActivity ? $this->l10n->t('You have added the label {label} to {card} in {stack} on {board}') : $this->l10n->t('{user} has added the label {label} to {card} in {stack} on {board}'); |
||
| 208 | break; |
||
| 209 | View Code Duplication | case self::SUBJECT_LABEL_UNASSING: |
|
| 210 | $subject = $ownActivity ? $this->l10n->t('You have removed the label {label} from {card} in {stack} on {board}') : $this->l10n->t('{user} has removed the label {label} from {card} in {stack} on {board}'); |
||
| 211 | break; |
||
| 212 | View Code Duplication | case self::SUBJECT_CARD_USER_ASSIGN: |
|
| 213 | $subject = $ownActivity ? $this->l10n->t('You have assigned {assigneduser} to {card} on {board}') : $this->l10n->t('{user} has assigned {assigneduser} to {card} on {board}'); |
||
| 214 | break; |
||
| 215 | View Code Duplication | case self::SUBJECT_CARD_USER_UNASSIGN: |
|
| 216 | $subject = $ownActivity ? $this->l10n->t('You have unassigned {assigneduser} from {card} on {board}') : $this->l10n->t('{user} has unassigned {assigneduser} from {card} on {board}'); |
||
| 217 | break; |
||
| 218 | View Code Duplication | case self::SUBJECT_CARD_UPDATE_STACKID: |
|
| 219 | $subject = $ownActivity ? $this->l10n->t('You have moved the card {card} from {stackBefore} to {stack}') : $this->l10n->t('{user} has moved the card {card} from {stackBefore} to {stack}'); |
||
| 220 | break; |
||
| 221 | View Code Duplication | case self::SUBJECT_ATTACHMENT_CREATE: |
|
| 222 | $subject = $ownActivity ? $this->l10n->t('You have added the attachment {attachment} to {card}') : $this->l10n->t('{user} has added the attachment {attachment} to {card}'); |
||
| 223 | break; |
||
| 224 | View Code Duplication | case self::SUBJECT_ATTACHMENT_UPDATE: |
|
| 225 | $subject = $ownActivity ? $this->l10n->t('You have updated the attachment {attachment} on {card}') : $this->l10n->t('{user} has updated the attachment {attachment} to {card}'); |
||
| 226 | break; |
||
| 227 | View Code Duplication | case self::SUBJECT_ATTACHMENT_DELETE: |
|
| 228 | $subject = $ownActivity ? $this->l10n->t('You have deleted the attachment {attachment} from {card}') : $this->l10n->t('{user} has deleted the attachment {attachment} to {card}'); |
||
| 229 | break; |
||
| 230 | View Code Duplication | case self::SUBJECT_ATTACHMENT_RESTORE: |
|
| 231 | $subject = $ownActivity ? $this->l10n->t('You have restored the attachment {attachment} to {card}') : $this->l10n->t('{user} has restored the attachment {attachment} to {card}'); |
||
| 232 | break; |
||
| 233 | case self::SUBJECT_CARD_COMMENT_CREATE: |
||
| 234 | $subject = $ownActivity ? $this->l10n->t('You have commented on {card}') : $this->l10n->t('{user} has commented on {card}'); |
||
| 235 | break; |
||
| 236 | default: |
||
| 237 | break; |
||
| 238 | } |
||
| 239 | return $subject; |
||
| 240 | } |
||
| 241 | |||
| 242 | public function triggerEvent($objectType, $entity, $subject, $additionalParams = []) { |
||
| 250 | |||
| 251 | /** |
||
| 252 | * |
||
| 253 | * @param $objectType |
||
| 254 | * @param ChangeSet $changeSet |
||
| 255 | * @param $subject |
||
| 256 | * @throws \Exception |
||
| 257 | */ |
||
| 258 | public function triggerUpdateEvents($objectType, ChangeSet $changeSet, $subject) { |
||
| 290 | |||
| 291 | /** |
||
| 292 | * @param $objectType |
||
| 293 | * @param $entity |
||
| 294 | * @param $subject |
||
| 295 | * @param array $additionalParams |
||
| 296 | * @return IEvent |
||
| 297 | * @throws \Exception |
||
| 298 | */ |
||
| 299 | private function createEvent($objectType, $entity, $subject, $additionalParams = []) { |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Publish activity to all users that are part of the board of a given object |
||
| 395 | * |
||
| 396 | * @param IEvent $event |
||
| 397 | */ |
||
| 398 | private function sendToUsers(IEvent $event) { |
||
| 415 | |||
| 416 | /** |
||
| 417 | * @param $objectType |
||
| 418 | * @param $entity |
||
| 419 | * @return null|\OCA\Deck\Db\RelationalEntity|\OCP\AppFramework\Db\Entity |
||
| 420 | * @throws \OCP\AppFramework\Db\DoesNotExistException |
||
| 421 | * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException |
||
| 422 | */ |
||
| 423 | private function findObjectForEntity($objectType, $entity) { |
||
| 464 | |||
| 465 | View Code Duplication | private function findDetailsForStack($stackId) { |
|
| 473 | |||
| 474 | private function findDetailsForCard($cardId) { |
||
| 484 | |||
| 485 | private function findDetailsForAttachment($attachmentId) { |
||
| 490 | |||
| 491 | View Code Duplication | private function findDetailsForAcl($aclId) { |
|
| 499 | |||
| 500 | } |
||
| 501 |
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.