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 Card 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 Card, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | class Card extends AbstractObject implements CardInterface |
||
13 | { |
||
14 | protected $apiName = 'card'; |
||
15 | |||
16 | protected $loadParams = array( |
||
17 | 'fields' => 'all', |
||
18 | 'board' => true, |
||
19 | 'list' => true, |
||
20 | 'stickers' => true, |
||
21 | 'members' => true, |
||
22 | 'membersVoted' => true, |
||
23 | 'attachments' => true, |
||
24 | 'checklists' => 'all', |
||
25 | 'checkItemStates' => true, |
||
26 | 'actions' => Events::CARD_COMMENT, |
||
27 | ); |
||
28 | |||
29 | protected $newChecklists = array(); |
||
30 | protected $newComments = array(); |
||
31 | protected $commentsToBeRemoved = array(); |
||
32 | |||
33 | /** |
||
34 | * {@inheritdoc} |
||
35 | */ |
||
36 | public function getShortId() |
||
40 | |||
41 | /** |
||
42 | * {@inheritdoc} |
||
43 | */ |
||
44 | public function setName($name) |
||
50 | |||
51 | /** |
||
52 | * {@inheritdoc} |
||
53 | */ |
||
54 | public function getName() |
||
58 | |||
59 | /** |
||
60 | * {@inheritdoc} |
||
61 | */ |
||
62 | public function setDescription($desc) |
||
68 | |||
69 | /** |
||
70 | * {@inheritdoc} |
||
71 | */ |
||
72 | public function getDescription() |
||
76 | |||
77 | /** |
||
78 | * {@inheritdoc} |
||
79 | */ |
||
80 | public function getDescriptionData() |
||
84 | |||
85 | /** |
||
86 | * {@inheritdoc} |
||
87 | */ |
||
88 | public function getUrl() |
||
92 | |||
93 | /** |
||
94 | * {@inheritdoc} |
||
95 | */ |
||
96 | public function getShortUrl() |
||
100 | |||
101 | /** |
||
102 | * {@inheritdoc} |
||
103 | */ |
||
104 | public function getShortLink() |
||
108 | |||
109 | /** |
||
110 | * {@inheritdoc} |
||
111 | */ |
||
112 | public function setPosition($pos) |
||
118 | |||
119 | /** |
||
120 | * {@inheritdoc} |
||
121 | */ |
||
122 | public function getPosition() |
||
126 | |||
127 | /** |
||
128 | * {@inheritdoc} |
||
129 | */ |
||
130 | public function setDueDate(\DateTime $due = null) |
||
136 | |||
137 | /** |
||
138 | * {@inheritdoc} |
||
139 | */ |
||
140 | public function getDueDate() |
||
148 | |||
149 | /** |
||
150 | * {@inheritdoc} |
||
151 | */ |
||
152 | public function setEmail($email) |
||
158 | |||
159 | /** |
||
160 | * {@inheritdoc} |
||
161 | */ |
||
162 | public function getEmail() |
||
166 | |||
167 | /** |
||
168 | * {@inheritdoc} |
||
169 | */ |
||
170 | public function setClosed($closed) |
||
176 | |||
177 | /** |
||
178 | * {@inheritdoc} |
||
179 | */ |
||
180 | public function isClosed() |
||
184 | |||
185 | /** |
||
186 | * {@inheritdoc} |
||
187 | */ |
||
188 | public function setSubscribed($subscribed) |
||
194 | |||
195 | /** |
||
196 | * {@inheritdoc} |
||
197 | */ |
||
198 | public function isSubscribed() |
||
202 | |||
203 | /** |
||
204 | * {@inheritdoc} |
||
205 | */ |
||
206 | public function setCheckItemStates(array $checkItemStates) |
||
212 | |||
213 | /** |
||
214 | * {@inheritdoc} |
||
215 | */ |
||
216 | public function getCheckItemStates() |
||
220 | |||
221 | /** |
||
222 | * {@inheritdoc} |
||
223 | */ |
||
224 | public function setBoardId($boardId) |
||
230 | |||
231 | /** |
||
232 | * {@inheritdoc} |
||
233 | */ |
||
234 | public function getBoardId() |
||
238 | |||
239 | /** |
||
240 | * {@inheritdoc} |
||
241 | */ |
||
242 | public function setBoard(BoardInterface $board) |
||
246 | |||
247 | /** |
||
248 | * {@inheritdoc} |
||
249 | */ |
||
250 | public function getBoard() |
||
254 | |||
255 | /** |
||
256 | * {@inheritdoc} |
||
257 | */ |
||
258 | public function setListId($listId) |
||
264 | |||
265 | /** |
||
266 | * {@inheritdoc} |
||
267 | */ |
||
268 | public function getListId() |
||
272 | |||
273 | /** |
||
274 | * {@inheritdoc} |
||
275 | */ |
||
276 | public function setList(CardlistInterface $list) |
||
280 | |||
281 | /** |
||
282 | * {@inheritdoc} |
||
283 | */ |
||
284 | public function getList() |
||
288 | |||
289 | public function moveToList($name) |
||
306 | |||
307 | /** |
||
308 | * {@inheritdoc} |
||
309 | */ |
||
310 | public function setChecklistIds(array $checklistIds) |
||
316 | |||
317 | /** |
||
318 | * {@inheritdoc} |
||
319 | */ |
||
320 | public function getChecklistIds() |
||
324 | |||
325 | /** |
||
326 | * {@inheritdoc} |
||
327 | */ |
||
328 | public function setChecklists(array $checklists) |
||
338 | |||
339 | /** |
||
340 | * {@inheritdoc} |
||
341 | */ |
||
342 | public function getChecklists() |
||
354 | |||
355 | /** |
||
356 | * {@inheritdoc} |
||
357 | * @param string $name |
||
358 | */ |
||
359 | public function getChecklist($name) |
||
373 | |||
374 | /** |
||
375 | * {@inheritdoc} |
||
376 | */ |
||
377 | public function hasChecklist($checklist) |
||
391 | |||
392 | /** |
||
393 | * {@inheritdoc} |
||
394 | */ |
||
395 | public function addChecklist($checklist) |
||
427 | |||
428 | /** |
||
429 | * {@inheritdoc} |
||
430 | */ |
||
431 | public function removeChecklist($checklist) |
||
454 | |||
455 | /** |
||
456 | * {@inheritdoc} |
||
457 | */ |
||
458 | public function setMemberIds(array $memberIds) |
||
464 | |||
465 | /** |
||
466 | * {@inheritdoc} |
||
467 | */ |
||
468 | public function getMemberIds() |
||
472 | |||
473 | /** |
||
474 | * {@inheritdoc} |
||
475 | */ |
||
476 | public function hasMember(MemberInterface $member) |
||
480 | |||
481 | /** |
||
482 | * {@inheritdoc} |
||
483 | */ |
||
484 | View Code Duplication | public function addMember(MemberInterface $member) |
|
498 | |||
499 | /** |
||
500 | * {@inheritdoc} |
||
501 | */ |
||
502 | View Code Duplication | public function removeMember(MemberInterface $member) |
|
520 | |||
521 | /** |
||
522 | * {@inheritdoc} |
||
523 | */ |
||
524 | public function setMembers(array $members) |
||
534 | |||
535 | /** |
||
536 | * {@inheritdoc} |
||
537 | */ |
||
538 | public function getMembers() |
||
548 | |||
549 | /** |
||
550 | * {@inheritdoc} |
||
551 | */ |
||
552 | public function setMembersVotedIds(array $membersVotedIds) |
||
558 | |||
559 | /** |
||
560 | * {@inheritdoc} |
||
561 | */ |
||
562 | public function getMembersVotedIds() |
||
566 | |||
567 | /** |
||
568 | * {@inheritdoc} |
||
569 | */ |
||
570 | public function hasMemberVoted(MemberInterface $member) |
||
574 | |||
575 | /** |
||
576 | * {@inheritdoc} |
||
577 | */ |
||
578 | View Code Duplication | public function addMemberVoted(MemberInterface $member) |
|
592 | |||
593 | /** |
||
594 | * {@inheritdoc} |
||
595 | */ |
||
596 | View Code Duplication | public function removeMemberVoted(MemberInterface $member) |
|
614 | |||
615 | /** |
||
616 | * {@inheritdoc} |
||
617 | */ |
||
618 | public function setMembersVoted(array $members) |
||
628 | |||
629 | /** |
||
630 | * {@inheritdoc} |
||
631 | */ |
||
632 | public function getMembersVoted() |
||
642 | |||
643 | /** |
||
644 | * {@inheritdoc} |
||
645 | */ |
||
646 | public function setAttachmentCoverId($attachmentCoverId) |
||
652 | |||
653 | /** |
||
654 | * {@inheritdoc} |
||
655 | */ |
||
656 | public function getAttachmentCoverId() |
||
660 | |||
661 | /** |
||
662 | * {@inheritdoc} |
||
663 | */ |
||
664 | public function setManualCoverAttachment($coverAttachment) |
||
670 | |||
671 | /** |
||
672 | * {@inheritdoc} |
||
673 | */ |
||
674 | public function getManualCoverAttachment() |
||
678 | |||
679 | /** |
||
680 | * {@inheritdoc} |
||
681 | */ |
||
682 | public function setLabels(array $labels) |
||
688 | |||
689 | /** |
||
690 | * {@inheritdoc} |
||
691 | */ |
||
692 | public function getLabels() |
||
696 | |||
697 | /** |
||
698 | * {@inheritdoc} |
||
699 | */ |
||
700 | public function getLabelColors() |
||
706 | |||
707 | /** |
||
708 | * {@inheritdoc} |
||
709 | */ |
||
710 | public function hasLabel($color) |
||
714 | |||
715 | /** |
||
716 | * {@inheritdoc} |
||
717 | */ |
||
718 | public function addLabel($color) |
||
732 | |||
733 | /** |
||
734 | * {@inheritdoc} |
||
735 | */ |
||
736 | public function removeLabel($color) |
||
754 | |||
755 | /** |
||
756 | * {@inheritdoc} |
||
757 | */ |
||
758 | public function setBadges(array $badges) |
||
764 | |||
765 | /** |
||
766 | * {@inheritdoc} |
||
767 | */ |
||
768 | public function getBadges() |
||
772 | |||
773 | /** |
||
774 | * {@inheritdoc} |
||
775 | */ |
||
776 | public function getDateOfLastActivity() |
||
780 | |||
781 | /** |
||
782 | * {@inheritdoc} |
||
783 | */ |
||
784 | public function getActions($params = array()) |
||
788 | |||
789 | /** |
||
790 | * {@inheritdoc} |
||
791 | */ |
||
792 | public function addComment($text) |
||
798 | |||
799 | /** |
||
800 | * {@inheritdoc} |
||
801 | */ |
||
802 | public function removeComment($commentId) |
||
808 | |||
809 | /** |
||
810 | * {@inheritdoc} |
||
811 | */ |
||
812 | protected function preSave() |
||
819 | |||
820 | /** |
||
821 | * {@inheritdoc} |
||
822 | */ |
||
823 | protected function postSave() |
||
835 | } |
||
836 |
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.