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 Account 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 Account, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
60 | class Account implements IAccount { |
||
61 | |||
62 | /** @var MailAccount */ |
||
63 | private $account; |
||
64 | |||
65 | /** @var Mailbox[]|null */ |
||
66 | private $mailboxes; |
||
67 | |||
68 | /** @var Horde_Imap_Client_Socket */ |
||
69 | private $client; |
||
70 | |||
71 | /** @var ICrypto */ |
||
72 | private $crypto; |
||
73 | |||
74 | /** @var IConfig */ |
||
75 | private $config; |
||
76 | |||
77 | /** @var ICacheFactory */ |
||
78 | private $memcacheFactory; |
||
79 | |||
80 | /** @var Alias */ |
||
81 | private $alias; |
||
82 | |||
83 | /** |
||
84 | * @param MailAccount $account |
||
85 | */ |
||
86 | 3 | public function __construct(MailAccount $account) { |
|
87 | 3 | $this->account = $account; |
|
88 | 3 | $this->mailboxes = null; |
|
89 | 3 | $this->crypto = \OC::$server->getCrypto(); |
|
90 | 3 | $this->config = \OC::$server->getConfig(); |
|
91 | 3 | $this->memcacheFactory = \OC::$server->getMemcacheFactory(); |
|
92 | 3 | $this->alias = null; |
|
93 | 3 | } |
|
94 | |||
95 | /** |
||
96 | * @return int |
||
97 | */ |
||
98 | 6 | public function getId() { |
|
101 | |||
102 | /** |
||
103 | * @param Alias $alias |
||
104 | * @return void |
||
105 | */ |
||
106 | public function setAlias($alias) { |
||
107 | $this->alias = new Alias(); |
||
108 | $this->alias = $alias; |
||
109 | } |
||
110 | |||
111 | /** |
||
112 | * @return string |
||
113 | */ |
||
114 | public function getName() { |
||
115 | return $this->alias ? $this->alias->getName() : $this->account->getName(); |
||
116 | } |
||
117 | |||
118 | /** |
||
119 | * @return string |
||
120 | */ |
||
121 | 3 | public function getEMailAddress() { |
|
124 | |||
125 | /** |
||
126 | * @return Horde_Imap_Client_Socket |
||
127 | */ |
||
128 | 13 | public function getImapConnection() { |
|
162 | |||
163 | /** |
||
164 | * @param string $mailBox |
||
165 | * @return Mailbox |
||
166 | */ |
||
167 | 12 | public function createMailbox($mailBox) { |
|
168 | 12 | $conn = $this->getImapConnection(); |
|
169 | 12 | $conn->createMailbox($mailBox); |
|
170 | 12 | $this->mailboxes = null; |
|
171 | |||
172 | 12 | return $this->getMailbox($mailBox); |
|
173 | } |
||
174 | |||
175 | /** |
||
176 | * Send a new message or reply to an existing message |
||
177 | * |
||
178 | * @param IMessage $message |
||
179 | * @param int|null $draftUID |
||
180 | */ |
||
181 | public function sendMessage(IMessage $message, $draftUID) { |
||
182 | // build mime body |
||
183 | $from = new Horde_Mail_Rfc822_Address($message->getFrom()); |
||
184 | $from->personal = $this->getName(); |
||
185 | $headers = [ |
||
186 | 'From' => $from, |
||
187 | 'To' => $message->getToList(), |
||
188 | 'Cc' => $message->getCCList(), |
||
189 | 'Bcc' => $message->getBCCList(), |
||
190 | 'Subject' => $message->getSubject(), |
||
191 | ]; |
||
192 | |||
193 | if (!is_null($message->getRepliedMessage())) { |
||
194 | $headers['In-Reply-To'] = $message->getRepliedMessage()->getMessageId(); |
||
195 | } |
||
196 | |||
197 | $mail = new Horde_Mime_Mail(); |
||
198 | $mail->addHeaders($headers); |
||
199 | $mail->setBody($message->getContent()); |
||
200 | |||
201 | // Append attachments |
||
202 | foreach ($message->getAttachments() as $attachment) { |
||
203 | $mail->addMimePart($attachment); |
||
204 | } |
||
205 | |||
206 | // Send the message |
||
207 | $transport = $this->createTransport(); |
||
208 | $mail->send($transport, false, false); |
||
209 | |||
210 | // Save the message in the sent folder |
||
211 | $sentFolder = $this->getSentFolder(); |
||
212 | /** @var resource $raw */ |
||
213 | $raw = stream_get_contents($mail->getRaw()); |
||
214 | $sentFolder->saveMessage($raw, [ |
||
215 | Horde_Imap_Client::FLAG_SEEN |
||
216 | ]); |
||
217 | |||
218 | // Delete draft if one exists |
||
219 | if (!is_null($draftUID)) { |
||
220 | $draftsFolder = $this->getDraftsFolder(); |
||
221 | $draftsFolder->setMessageFlag($draftUID, Horde_Imap_Client::FLAG_DELETED, true); |
||
222 | $this->deleteDraft($draftUID); |
||
223 | } |
||
224 | } |
||
225 | |||
226 | /** |
||
227 | * @param IMessage $message |
||
228 | * @param int|null $previousUID |
||
229 | * @return int |
||
230 | */ |
||
231 | public function saveDraft(IMessage $message, $previousUID) { |
||
267 | |||
268 | /** |
||
269 | * @param string $mailBox |
||
270 | */ |
||
271 | 12 | public function deleteMailbox($mailBox) { |
|
279 | |||
280 | /** |
||
281 | * Lists mailboxes (folders) for this account. |
||
282 | * |
||
283 | * Lists mailboxes and also queries the server for their 'special use', |
||
284 | * eg. inbox, sent, trash, etc |
||
285 | * |
||
286 | * @param string $pattern Pattern to match mailboxes against. All by default. |
||
287 | * @return Mailbox[] |
||
288 | */ |
||
289 | 6 | protected function listMailboxes($pattern = '*') { |
|
314 | |||
315 | /** |
||
316 | * @param string $folderId |
||
317 | * @return \OCA\Mail\Mailbox |
||
318 | */ |
||
319 | 12 | public function getMailbox($folderId) { |
|
329 | |||
330 | /** |
||
331 | * Get a list of all mailboxes in this account |
||
332 | * |
||
333 | * @return Mailbox[] |
||
334 | */ |
||
335 | 7 | public function getMailboxes() { |
|
344 | |||
345 | /** |
||
346 | * @return array |
||
347 | */ |
||
348 | 3 | public function getListArray() { |
|
374 | |||
375 | /** |
||
376 | * @return Horde_Mail_Transport |
||
377 | */ |
||
378 | public function createTransport() { |
||
399 | |||
400 | /** |
||
401 | * Lists special use folders for this account. |
||
402 | * |
||
403 | * The special uses returned are the "best" one for each special role, |
||
404 | * picked amongst the ones returned by the server, as well |
||
405 | * as the one guessed by our code. |
||
406 | * |
||
407 | * @param bool $base64_encode |
||
408 | * @return array In the form [<special use>=><folder id>, ...] |
||
409 | */ |
||
410 | 6 | public function getSpecialFoldersIds($base64_encode=true) { |
|
423 | |||
424 | /** |
||
425 | * Get the "drafts" mailbox |
||
426 | * |
||
427 | * @return Mailbox The best candidate for the "drafts" inbox |
||
428 | */ |
||
429 | View Code Duplication | public function getDraftsFolder() { |
|
443 | |||
444 | /** |
||
445 | * @return IMailBox |
||
446 | */ |
||
447 | public function getInbox() { |
||
451 | |||
452 | /** |
||
453 | * Get the "sent mail" mailbox |
||
454 | * |
||
455 | * @return Mailbox The best candidate for the "sent mail" inbox |
||
456 | */ |
||
457 | View Code Duplication | public function getSentFolder() { |
|
471 | |||
472 | /** |
||
473 | * @param string $sourceFolderId |
||
474 | * @param int $messageId |
||
475 | */ |
||
476 | public function deleteMessage($sourceFolderId, $messageId) { |
||
520 | |||
521 | /** |
||
522 | * |
||
523 | * @param int $messageId |
||
524 | */ |
||
525 | public function deleteDraft($messageId) { |
||
531 | |||
532 | /** |
||
533 | * Get 'best' mailbox guess |
||
534 | * |
||
535 | * For now the best candidate is the one with |
||
536 | * the most messages in it. |
||
537 | * |
||
538 | * @param array $folders |
||
539 | * @return Mailbox |
||
540 | */ |
||
541 | protected function guessBestMailBox(array $folders) { |
||
553 | |||
554 | /** |
||
555 | * Get mailbox(es) that have the given special use role |
||
556 | * |
||
557 | * With this method we can get a list of all mailboxes that have been |
||
558 | * determined to have a specific special use role. It can also return |
||
559 | * the best candidate for this role, for situations where we want |
||
560 | * one single folder. |
||
561 | * |
||
562 | * @param string $role Special role of the folder we want to get ('sent', 'inbox', etc.) |
||
563 | * @param bool $guessBest If set to true, return only the folder with the most messages in it |
||
564 | * |
||
565 | * @return Mailbox[] if $guessBest is false, or Mailbox if $guessBest is true. Empty [] if no match. |
||
566 | */ |
||
567 | 6 | protected function getSpecialFolder($role, $guessBest=true) { |
|
582 | |||
583 | /** |
||
584 | * Localizes the name of the special use folders |
||
585 | * |
||
586 | * The display name of the best candidate folder for each special use |
||
587 | * is localized to the user's language |
||
588 | */ |
||
589 | 6 | protected function localizeSpecialMailboxes() { |
|
621 | |||
622 | /** |
||
623 | * Sort mailboxes |
||
624 | * |
||
625 | * Sort the array of mailboxes with |
||
626 | * - special use folders coming first in this order: all, inbox, flagged, drafts, sent, archive, junk, trash |
||
627 | * - 'normal' folders coming after that, sorted alphabetically |
||
628 | */ |
||
629 | 6 | protected function sortMailboxes() { |
|
675 | |||
676 | /** |
||
677 | * Convert special security mode values into Horde parameters |
||
678 | * |
||
679 | * @param string $sslMode |
||
680 | * @return false|string |
||
681 | */ |
||
682 | protected function convertSslMode($sslMode) { |
||
689 | |||
690 | /** |
||
691 | * @param $query |
||
692 | * @return array |
||
693 | */ |
||
694 | 4 | public function getChangedMailboxes($query) { |
|
741 | |||
742 | /** |
||
743 | * @throws \Horde_Imap_Client_Exception |
||
744 | */ |
||
745 | public function reconnect() { |
||
753 | |||
754 | /** |
||
755 | * @return array |
||
756 | */ |
||
757 | public function getConfiguration() { |
||
760 | |||
761 | /** |
||
762 | * @return string|Horde_Mail_Rfc822_List |
||
763 | */ |
||
764 | public function getEmail() { |
||
767 | |||
768 | public function testConnectivity() { |
||
778 | |||
779 | /** |
||
780 | * Factory method for creating new messages |
||
781 | * |
||
782 | * @return IMessage |
||
783 | */ |
||
784 | public function newMessage() { |
||
787 | |||
788 | /** |
||
789 | * Factory method for creating new reply messages |
||
790 | * |
||
791 | * @return ReplyMessage |
||
792 | */ |
||
793 | public function newReplyMessage() { |
||
796 | |||
797 | } |
||
798 |
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.