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:
1 | <?php |
||
53 | class AccountsController extends Controller { |
||
54 | |||
55 | /** @var AccountService */ |
||
56 | private $accountService; |
||
57 | |||
58 | /** @var string */ |
||
59 | private $currentUserId; |
||
60 | |||
61 | /** @var AutoConfig */ |
||
62 | private $autoConfig; |
||
63 | |||
64 | /** @var Folder */ |
||
65 | private $userFolder; |
||
66 | |||
67 | /** @var Logger */ |
||
68 | private $logger; |
||
69 | |||
70 | /** @var IL10N */ |
||
71 | private $l10n; |
||
72 | |||
73 | /** @var ICrypto */ |
||
74 | private $crypto; |
||
75 | |||
76 | /** @var AddressCollector */ |
||
77 | private $addressCollector; |
||
78 | |||
79 | /** @var AliasesService */ |
||
80 | private $aliasesService; |
||
81 | |||
82 | /** |
||
83 | * @param string $appName |
||
84 | * @param IRequest $request |
||
85 | * @param AccountService $accountService |
||
86 | * @param $UserId |
||
87 | * @param $userFolder |
||
88 | * @param AutoConfig $autoConfig |
||
89 | * @param Logger $logger |
||
90 | * @param IL10N $l10n |
||
91 | * @param ICrypto $crypto |
||
92 | */ |
||
93 | 15 | View Code Duplication | public function __construct($appName, |
|
|||
94 | IRequest $request, |
||
95 | AccountService $accountService, |
||
96 | $UserId, |
||
97 | $userFolder, |
||
98 | AutoConfig $autoConfig, |
||
99 | Logger $logger, |
||
100 | IL10N $l10n, |
||
101 | ICrypto $crypto, |
||
102 | AddressCollector $addressCollector, |
||
103 | AliasesService $aliasesService |
||
104 | ) { |
||
105 | 15 | parent::__construct($appName, $request); |
|
106 | 15 | $this->accountService = $accountService; |
|
107 | 15 | $this->currentUserId = $UserId; |
|
108 | 15 | $this->userFolder = $userFolder; |
|
109 | 15 | $this->autoConfig = $autoConfig; |
|
110 | 15 | $this->logger = $logger; |
|
111 | 15 | $this->l10n = $l10n; |
|
112 | 15 | $this->crypto = $crypto; |
|
113 | 15 | $this->addressCollector = $addressCollector; |
|
114 | 15 | $this->aliasesService = $aliasesService; |
|
115 | 15 | } |
|
116 | |||
117 | /** |
||
118 | * @NoAdminRequired |
||
119 | * @NoCSRFRequired |
||
120 | * |
||
121 | * @return JSONResponse |
||
122 | */ |
||
123 | 1 | public function index() { |
|
124 | 1 | $mailAccounts = $this->accountService->findByUserId($this->currentUserId); |
|
125 | |||
126 | 1 | $json = []; |
|
127 | 1 | View Code Duplication | foreach ($mailAccounts as $mailAccount) { |
128 | 1 | $conf = $mailAccount->getConfiguration(); |
|
129 | 1 | $conf['aliases'] = $this->aliasesService->findAll($conf['accountId'], $this->currentUserId); |
|
130 | 1 | $json[] = $conf; |
|
131 | 1 | } |
|
132 | 1 | return new JSONResponse($json); |
|
133 | } |
||
134 | |||
135 | /** |
||
136 | * @NoAdminRequired |
||
137 | * |
||
138 | * @param int $accountId |
||
139 | * @return JSONResponse |
||
140 | */ |
||
141 | 2 | public function show($accountId) { |
|
150 | |||
151 | /** |
||
152 | * @NoAdminRequired |
||
153 | */ |
||
154 | public function update() { |
||
159 | |||
160 | /** |
||
161 | * @NoAdminRequired |
||
162 | * |
||
163 | * @param int $accountId |
||
164 | * @return JSONResponse |
||
165 | */ |
||
166 | 2 | public function destroy($accountId) { |
|
175 | |||
176 | /** |
||
177 | * @NoAdminRequired |
||
178 | * |
||
179 | * @param string $accountName |
||
180 | * @param string $emailAddress |
||
181 | * @param string $password |
||
182 | * @param string $imapHost |
||
183 | * @param int $imapPort |
||
184 | * @param string $imapSslMode |
||
185 | * @param string $imapUser |
||
186 | * @param string $imapPassword |
||
187 | * @param string $smtpHost |
||
188 | * @param int $smtpPort |
||
189 | * @param string $smtpSslMode |
||
190 | * @param string $smtpUser |
||
191 | * @param string $smtpPassword |
||
192 | * @param bool $autoDetect |
||
193 | * @return JSONResponse |
||
194 | */ |
||
195 | 2 | public function create($accountName, $emailAddress, $password, |
|
255 | |||
256 | /** |
||
257 | * @NoAdminRequired |
||
258 | * |
||
259 | * @param int $accountId |
||
260 | * @param string $folderId |
||
261 | * @param string $subject |
||
262 | * @param string $body |
||
263 | * @param string $to |
||
264 | * @param string $cc |
||
265 | * @param string $bcc |
||
266 | * @param int $draftUID |
||
267 | * @param string $messageId |
||
268 | * @param mixed $attachments |
||
269 | * @return JSONResponse |
||
270 | */ |
||
271 | 4 | public function send($accountId, $folderId, $subject, $body, $to, $cc, |
|
272 | $bcc, $draftUID, $messageId, $attachments, $aliasId) { |
||
273 | 4 | $account = $this->accountService->find($this->currentUserId, $accountId); |
|
274 | 4 | $alias = $aliasId ? $this->aliasesService->find($aliasId, $this->currentUserId) : null; |
|
275 | 4 | if ($account instanceof UnifiedAccount) { |
|
276 | 2 | list($account, $folderId, $messageId) = $account->resolve($messageId); |
|
277 | 2 | } |
|
278 | 4 | if (!$account instanceof Account) { |
|
279 | return new JSONResponse( |
||
280 | ['message' => 'Invalid account'], |
||
281 | Http::STATUS_BAD_REQUEST |
||
282 | ); |
||
283 | } |
||
284 | |||
285 | 4 | $mailbox = null; |
|
286 | 4 | if (!is_null($folderId) && !is_null($messageId)) { |
|
287 | // Reply |
||
288 | 2 | $message = $account->newReplyMessage(); |
|
289 | |||
290 | 2 | $mailbox = $account->getMailbox(base64_decode($folderId)); |
|
291 | 2 | $repliedMessage = $mailbox->getMessage($messageId); |
|
292 | |||
293 | 2 | if (is_null($subject)) { |
|
294 | // No subject set – use the original one |
||
295 | $message->setSubject($repliedMessage->getSubject()); |
||
296 | } else { |
||
297 | 2 | $message->setSubject($subject); |
|
298 | } |
||
299 | |||
300 | 2 | if (is_null($to)) { |
|
301 | $message->setTo(Message::parseAddressList($repliedMessage->getToList())); |
||
302 | } else { |
||
303 | 2 | $message->setTo(Message::parseAddressList($to)); |
|
304 | } |
||
305 | |||
306 | 2 | $message->setRepliedMessage($repliedMessage); |
|
307 | 2 | } else { |
|
308 | // New message |
||
309 | 2 | $message = $account->newMessage(); |
|
310 | 2 | $message->setTo(Message::parseAddressList($to)); |
|
311 | 2 | $message->setSubject($subject ? : ''); |
|
312 | } |
||
313 | |||
314 | 4 | $account->setAlias($alias); |
|
315 | 4 | $message->setFrom($alias ? $alias->alias : $account->getEMailAddress()); |
|
316 | 4 | $message->setCC(Message::parseAddressList($cc)); |
|
317 | 4 | $message->setBcc(Message::parseAddressList($bcc)); |
|
318 | 4 | $message->setContent($body); |
|
319 | |||
320 | 4 | if (is_array($attachments)) { |
|
321 | 4 | foreach($attachments as $attachment) { |
|
322 | 4 | $fileName = $attachment['fileName']; |
|
323 | 4 | if ($this->userFolder->nodeExists($fileName)) { |
|
324 | 4 | $f = $this->userFolder->get($fileName); |
|
325 | 4 | if ($f instanceof File) { |
|
326 | 4 | $message->addAttachmentFromFiles($f); |
|
327 | 4 | } |
|
328 | 4 | } |
|
329 | 4 | } |
|
330 | 4 | } |
|
331 | |||
332 | try { |
||
333 | 4 | $account->sendMessage($message, $draftUID); |
|
334 | |||
335 | // in case of reply we flag the message as answered |
||
336 | 4 | if ($message instanceof ReplyMessage) { |
|
337 | 2 | $mailbox->setMessageFlag($messageId, Horde_Imap_Client::FLAG_ANSWERED, true); |
|
338 | 2 | } |
|
339 | |||
340 | // Collect mail addresses |
||
341 | try { |
||
342 | 4 | $addresses = array_merge($message->getToList(), $message->getCCList(), $message->getBCCList()); |
|
343 | 4 | $this->addressCollector->addAddresses($addresses); |
|
344 | 4 | } catch (Exception $e) { |
|
345 | 1 | $this->logger->error("Error while collecting mail addresses: " . $e->getMessage()); |
|
346 | } |
||
347 | 4 | } catch (Horde_Exception $ex) { |
|
348 | $this->logger->error('Sending mail failed: ' . $ex->getMessage()); |
||
349 | return new JSONResponse( |
||
350 | array('message' => $ex->getMessage()), |
||
351 | Http::STATUS_INTERNAL_SERVER_ERROR |
||
352 | ); |
||
353 | } |
||
354 | |||
355 | 4 | return new JSONResponse(); |
|
356 | } |
||
357 | |||
358 | /** |
||
359 | * @NoAdminRequired |
||
360 | * |
||
361 | * @param int $accountId |
||
362 | * @param string $subject |
||
363 | * @param string $body |
||
364 | * @param string $to |
||
365 | * @param string $cc |
||
366 | * @param string $bcc |
||
367 | * @param int $uid |
||
368 | * @param string $messageId |
||
369 | * @return JSONResponse |
||
370 | */ |
||
371 | 4 | public function draft($accountId, $subject, $body, $to, $cc, $bcc, $uid, $messageId) { |
|
414 | |||
415 | } |
||
416 |
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.