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 EmailApiHandler 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 EmailApiHandler, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | class EmailApiHandler extends ApiFormHandler |
||
32 | { |
||
33 | /** @var EmailEntityBuilder */ |
||
34 | protected $emailEntityBuilder; |
||
35 | |||
36 | /** @var SecurityFacade */ |
||
37 | protected $securityFacade; |
||
38 | |||
39 | /** @var EventDispatcherInterface */ |
||
40 | protected $eventDispatcher; |
||
41 | |||
42 | /** @var DataTransformerInterface */ |
||
43 | protected $emailImportanceTransformer; |
||
44 | |||
45 | /** @var DataTransformerInterface */ |
||
46 | protected $emailBodyTypeTransformer; |
||
47 | |||
48 | /** @var EmailOrigin|null */ |
||
49 | protected $emailOrigin; |
||
50 | |||
51 | /** |
||
52 | * @param FormInterface $form |
||
53 | * @param Request $request |
||
54 | * @param EntityManager $entityManager |
||
55 | * @param EmailEntityBuilder $emailEntityBuilder |
||
56 | * @param SecurityFacade $securityFacade |
||
57 | * @param EventDispatcherInterface $eventDispatcher |
||
58 | * @param DataTransformerInterface $emailImportanceTransformer |
||
59 | * @param DataTransformerInterface $emailBodyTypeTransformer |
||
60 | */ |
||
61 | public function __construct( |
||
62 | FormInterface $form, |
||
63 | Request $request, |
||
64 | EntityManager $entityManager, |
||
65 | EmailEntityBuilder $emailEntityBuilder, |
||
66 | SecurityFacade $securityFacade, |
||
67 | EventDispatcherInterface $eventDispatcher, |
||
68 | DataTransformerInterface $emailImportanceTransformer, |
||
69 | DataTransformerInterface $emailBodyTypeTransformer |
||
70 | ) { |
||
71 | parent::__construct($form, $request, $entityManager); |
||
72 | |||
73 | $this->emailEntityBuilder = $emailEntityBuilder; |
||
74 | $this->securityFacade = $securityFacade; |
||
75 | $this->eventDispatcher = $eventDispatcher; |
||
76 | $this->emailImportanceTransformer = $emailImportanceTransformer; |
||
77 | $this->emailBodyTypeTransformer = $emailBodyTypeTransformer; |
||
78 | } |
||
79 | |||
80 | /** |
||
81 | * @param Email $entity |
||
82 | * |
||
83 | * @return EmailModel |
||
84 | */ |
||
85 | protected function prepareFormData($entity) |
||
89 | |||
90 | /** |
||
91 | * @param EmailModel $entity |
||
92 | * |
||
93 | * @return Email |
||
94 | */ |
||
95 | protected function onSuccess($entity) |
||
96 | { |
||
97 | $this->processEmailModel($entity); |
||
98 | $this->entityManager->flush(); |
||
99 | |||
100 | if ($entity->getBody()) { |
||
101 | $this->eventDispatcher->dispatch( |
||
102 | EmailBodyAdded::NAME, |
||
103 | new EmailBodyAdded($entity->getEntity()) |
||
|
|||
104 | ); |
||
105 | } |
||
106 | |||
107 | return $entity->getEntity(); |
||
108 | } |
||
109 | |||
110 | /** |
||
111 | * @param EmailModel $model |
||
112 | * |
||
113 | * @SuppressWarnings(PHPMD.NPathComplexity) |
||
114 | * @SuppressWarnings(PHPMD.CyclomaticComplexity) |
||
115 | * @SuppressWarnings(PHPMD.ExcessiveMethodLength) |
||
116 | */ |
||
117 | protected function processEmailModel(EmailModel $model) |
||
222 | |||
223 | /** |
||
224 | * @param EmailModel $model |
||
225 | * |
||
226 | * @throws \InvalidArgumentException |
||
227 | */ |
||
228 | protected function assertModel(EmailModel $model) |
||
229 | { |
||
230 | if ($model->getEntity() && $model->getEntity()->getId()) { |
||
231 | return; |
||
232 | } |
||
233 | |||
234 | if (!$model->getFolders()) { |
||
235 | throw new \InvalidArgumentException('Folders should not be empty'); |
||
236 | } |
||
237 | if (!$model->getMessageId()) { |
||
238 | throw new \InvalidArgumentException('Message-ID should not be empty'); |
||
239 | } |
||
240 | if (!$model->getFrom()) { |
||
241 | throw new \InvalidArgumentException('Sender should not be empty'); |
||
242 | } |
||
243 | View Code Duplication | if (!$model->getTo() && !$model->getCc() && !$model->getBcc()) { |
|
244 | throw new \InvalidArgumentException('Recipients should not be empty'); |
||
245 | } |
||
246 | } |
||
247 | |||
248 | /** |
||
249 | * @param EmailModel $model |
||
250 | */ |
||
251 | protected function ensureEmailEntitySet(EmailModel $model) |
||
252 | { |
||
253 | if ($model->getEntity() && $model->getEntity()->getId()) { |
||
254 | return; |
||
255 | } |
||
256 | |||
257 | /** @var EmailRepository $repo */ |
||
258 | $repo = $this->entityManager->getRepository('Oro\Bundle\EmailBundle\Entity\Email'); |
||
259 | $entity = $repo->findEmailByMessageId($model->getMessageId()); |
||
260 | if ($entity) { |
||
261 | $model->setEntity($entity); |
||
262 | } |
||
263 | if (!$model->getEntity()) { |
||
264 | $model->setEntity(new Email()); |
||
265 | } |
||
266 | } |
||
267 | |||
268 | /** |
||
269 | * @return EmailOrigin |
||
270 | */ |
||
271 | protected function getEmailOrigin() |
||
272 | { |
||
273 | if (!$this->emailOrigin) { |
||
274 | /** @var User $originOwner */ |
||
275 | $originOwner = $this->securityFacade->getLoggedUser(); |
||
276 | $organization = $this->securityFacade->getOrganization(); |
||
277 | $originName = InternalEmailOrigin::BAP . '_User_' . $originOwner->getId(); |
||
278 | |||
279 | $origins = $originOwner->getEmailOrigins()->filter( |
||
280 | function ($item) use ($originName, $organization) { |
||
281 | return |
||
282 | $item instanceof InternalEmailOrigin |
||
283 | && $item->getOrganization() === $organization; |
||
284 | } |
||
285 | ); |
||
286 | |||
287 | $this->emailOrigin = !$origins->isEmpty() |
||
288 | ? $origins->first() |
||
289 | : $this->createEmailOrigin($originOwner, $organization, $originName); |
||
290 | } |
||
291 | |||
292 | return $this->emailOrigin; |
||
293 | } |
||
294 | |||
295 | /** |
||
296 | * @param User $originOwner |
||
297 | * @param OrganizationInterface $organization |
||
298 | * @param string $originName |
||
299 | * |
||
300 | * @return InternalEmailOrigin |
||
301 | */ |
||
302 | protected function createEmailOrigin(User $originOwner, OrganizationInterface $organization, $originName) |
||
303 | { |
||
304 | $origin = new InternalEmailOrigin(); |
||
305 | $origin->setName($originName); |
||
306 | $origin->setOwner($originOwner); |
||
307 | $origin->setOrganization($organization); |
||
308 | |||
309 | $originOwner->addEmailOrigin($origin); |
||
310 | |||
311 | $this->entityManager->persist($origin); |
||
312 | |||
313 | return $origin; |
||
314 | } |
||
315 | |||
316 | /** |
||
317 | * @param Email $email |
||
318 | * @param array $folders |
||
319 | * |
||
320 | * @return EmailUser[] |
||
321 | */ |
||
322 | protected function processFolders(Email $email, $folders) |
||
323 | { |
||
324 | $apiOrigin = $this->getEmailOrigin(); |
||
325 | $emailUserList = []; |
||
326 | foreach ($folders as $item) { |
||
327 | $origin = $item['origin'] ?: $this->getEmailOrigin(); |
||
328 | if ($origin->getId() && $origin->getId() !== $apiOrigin->getId()) { |
||
329 | continue; |
||
330 | } |
||
331 | |||
332 | $folder = $origin->getFolder($item['type'], $item['fullName']); |
||
333 | if (!$folder) { |
||
334 | $folder = $this->emailEntityBuilder->folder($item['type'], $item['fullName'], $item['name']); |
||
335 | $origin->addFolder($folder); |
||
336 | } else { |
||
337 | $this->emailEntityBuilder->setFolder($folder); |
||
338 | } |
||
339 | |||
340 | $emailUser = new EmailUser(); |
||
341 | $emailUser->setEmail($email); |
||
342 | $emailUser->setOwner($apiOrigin->getOwner()); |
||
343 | $emailUser->setOrganization($apiOrigin->getOrganization()); |
||
344 | $emailUser->addFolder($folder); |
||
345 | $emailUser->setOrigin($origin); |
||
346 | |||
347 | $emailUserList[] = $emailUser; |
||
348 | } |
||
349 | |||
350 | return $emailUserList; |
||
351 | } |
||
352 | |||
353 | /** |
||
354 | * @param Email $email |
||
355 | * @param string $property |
||
356 | * @param string $value |
||
357 | */ |
||
358 | protected function processString(Email $email, $property, $value) |
||
359 | { |
||
360 | if ($email->getId()) { |
||
361 | if ($email->{'get' . $property}() !== $value) { |
||
362 | throw $this->createInvalidPropertyException( |
||
363 | $property, |
||
364 | $email->{'get' . $property}(), |
||
365 | $value |
||
366 | ); |
||
367 | } |
||
368 | } else { |
||
369 | $email->{'set' . $property}($value); |
||
370 | } |
||
371 | } |
||
372 | |||
373 | /** |
||
374 | * @param Email $email |
||
375 | * @param string $sender |
||
376 | */ |
||
377 | protected function processSender(Email $email, $sender) |
||
378 | { |
||
379 | if ($email->getId()) { |
||
380 | if (strtolower($email->getFromName()) !== strtolower($sender)) { |
||
381 | throw $this->createInvalidPropertyException( |
||
382 | 'Sender', |
||
383 | $email->getFromName(), |
||
384 | $sender |
||
385 | ); |
||
386 | } |
||
387 | } else { |
||
388 | |||
389 | ->setFromName($sender) |
||
390 | ->setFromEmailAddress($this->emailEntityBuilder->address($sender)); |
||
391 | } |
||
392 | } |
||
393 | |||
394 | /** |
||
395 | * @param Email $email |
||
396 | * @param string $type |
||
397 | * @param string[] $recipients |
||
398 | */ |
||
399 | protected function processRecipients(Email $email, $type, array $recipients) |
||
416 | |||
417 | /** |
||
418 | * @param Email $email |
||
419 | * @param string|null $content |
||
420 | * @param bool|null $type |
||
421 | */ |
||
422 | protected function processBody(Email $email, $content, $type) |
||
423 | { |
||
424 | $body = $email->getEmailBody(); |
||
425 | if ($body) { |
||
426 | if ($email->getId()) { |
||
427 | if ($body->getBodyContent() !== $content) { |
||
428 | throw $this->createInvalidPropertyException( |
||
429 | 'Body Content', |
||
430 | $body->getBodyContent(), |
||
431 | $content |
||
432 | ); |
||
433 | } |
||
434 | } else { |
||
435 | $body->setBodyContent($content); |
||
436 | } |
||
437 | View Code Duplication | if ($email->getId()) { |
|
438 | if ($body->getBodyIsText() !== ($type === true)) { |
||
439 | throw $this->createInvalidPropertyException( |
||
440 | 'Body Type', |
||
441 | $this->emailBodyTypeTransformer->transform($body->getBodyIsText()), |
||
442 | $this->emailBodyTypeTransformer->transform($type) |
||
443 | ); |
||
444 | } |
||
445 | } else { |
||
446 | $body->setBodyIsText($type === true); |
||
447 | } |
||
448 | } else { |
||
449 | $email->setEmailBody($this->emailEntityBuilder->body($content, $type !== true, true)); |
||
450 | } |
||
451 | } |
||
452 | |||
453 | /** |
||
454 | * @param Email $email |
||
455 | * @param bool $type |
||
456 | */ |
||
457 | protected function processBodyType(Email $email, $type) |
||
476 | |||
477 | /** |
||
478 | * @param Email $email |
||
479 | * @param string $importance |
||
480 | */ |
||
481 | protected function processImportance(Email $email, $importance) |
||
482 | { |
||
483 | if ($email->getId()) { |
||
484 | if ($email->getImportance() != $importance) { |
||
485 | throw $this->createInvalidPropertyException( |
||
486 | 'Importance', |
||
487 | $this->emailImportanceTransformer->transform($email->getImportance()), |
||
488 | $this->emailImportanceTransformer->transform($importance) |
||
489 | ); |
||
490 | } |
||
491 | } else { |
||
492 | $email->setImportance($importance); |
||
493 | } |
||
494 | } |
||
495 | |||
496 | /** |
||
497 | * @param Email $email |
||
498 | * @param bool $head |
||
499 | */ |
||
500 | protected function processHead(Email $email, $head) |
||
501 | { |
||
502 | if ($email->getId()) { |
||
503 | if ($email->isHead() != $head) { |
||
504 | throw $this->createInvalidPropertyException( |
||
505 | 'Head', |
||
506 | $email->isHead() ? 'true' : 'false', |
||
507 | $head ? 'true' : 'false' |
||
508 | ); |
||
509 | } |
||
510 | } else { |
||
511 | $email->setHead($head); |
||
512 | } |
||
513 | } |
||
514 | |||
515 | /** |
||
516 | * @param Email $email |
||
517 | * @param EmailThread $thread |
||
518 | */ |
||
519 | protected function processThread(Email $email, EmailThread $thread) |
||
520 | { |
||
521 | if ($email->getId()) { |
||
522 | if (!$email->getThread() || $email->getThread()->getId() != $thread->getId()) { |
||
523 | throw $this->createInvalidPropertyException( |
||
524 | 'Thread', |
||
525 | $email->getThread() ? $email->getThread()->getId() : null, |
||
526 | $thread->getId() |
||
527 | ); |
||
528 | } |
||
529 | } else { |
||
530 | $email->setThread($thread); |
||
531 | } |
||
532 | } |
||
533 | |||
534 | /** |
||
535 | * @param Email $email |
||
536 | * @param string $refs |
||
537 | */ |
||
538 | protected function processRefs(Email $email, $refs) |
||
539 | { |
||
540 | if ($email->getId()) { |
||
541 | if (!$this->areRefsEqual($email->getRefs(), $refs)) { |
||
542 | throw $this->createInvalidPropertyException( |
||
543 | 'Refs', |
||
544 | $this->convertRefsToString($email->getRefs()), |
||
545 | $this->convertRefsToString($refs) |
||
546 | ); |
||
547 | } |
||
548 | } else { |
||
549 | $email->setRefs($refs); |
||
550 | } |
||
551 | } |
||
552 | |||
553 | /** |
||
554 | * @param string[]|EmailRecipient[]|Collection $existingRecipients |
||
555 | * @param string[]|EmailRecipient[]|Collection $newRecipients |
||
556 | * |
||
557 | * @return bool |
||
558 | */ |
||
559 | protected function areRecipientsEqual($existingRecipients, $newRecipients) |
||
576 | |||
577 | /** |
||
578 | * Returns alphabetically sorted array of string representation of each recipient in the given list |
||
579 | * |
||
580 | * @param string[]|EmailRecipient[]|Collection $recipients |
||
581 | * |
||
582 | * @return string[] |
||
583 | */ |
||
584 | protected function normalizeRecipients($recipients) |
||
585 | { |
||
586 | if ($recipients instanceof Collection) { |
||
587 | $recipients = $recipients->toArray(); |
||
588 | } |
||
589 | if (reset($recipients) instanceof EmailRecipient) { |
||
590 | $recipients = array_map( |
||
591 | function (EmailRecipient $recipient) { |
||
592 | return $recipient->getName(); |
||
593 | }, |
||
594 | $recipients |
||
595 | ); |
||
596 | } |
||
597 | sort($recipients, SORT_STRING | SORT_FLAG_CASE); |
||
598 | |||
599 | return array_values($recipients); |
||
600 | } |
||
601 | |||
602 | /** |
||
603 | * Returns human readable representation of the recipient list |
||
604 | * |
||
605 | * @param string[]|EmailRecipient[]|Collection $recipients |
||
606 | * |
||
607 | * @return string |
||
608 | */ |
||
609 | protected function convertRecipientsToString($recipients) |
||
613 | |||
614 | /** |
||
615 | * @param string[]|string $existingRefs |
||
616 | * @param string[]|string $newRefs |
||
617 | * |
||
618 | * @return bool |
||
619 | */ |
||
620 | protected function areRefsEqual($existingRefs, $newRefs) |
||
637 | |||
638 | /** |
||
639 | * Returns alphabetically sorted array of string representation of each reference in the given list |
||
640 | * |
||
641 | * @param string[]|string $refs |
||
642 | * |
||
643 | * @return string[] |
||
644 | */ |
||
645 | protected function normalizeRefs($refs) |
||
661 | |||
662 | /** |
||
663 | * Returns human readable representation of the reference list |
||
664 | * |
||
665 | * @param string[]|string $refs |
||
666 | * |
||
667 | * @return string |
||
668 | */ |
||
669 | protected function convertRefsToString($refs) |
||
673 | |||
674 | /** |
||
675 | * @param string $property |
||
676 | * @param mixed $existingValue |
||
677 | * @param mixed $newValue |
||
678 | * |
||
679 | * @return \InvalidArgumentException |
||
680 | */ |
||
681 | protected function createInvalidPropertyException($property, $existingValue, $newValue) |
||
682 | { |
||
683 | return new \InvalidArgumentException( |
||
684 | sprintf( |
||
685 | 'The %s cannot be changed for already existing email.' |
||
686 | . ' Existing value: "%s". New value: "%s".', |
||
687 | $property, |
||
688 | $existingValue, |
||
693 | } |
||
694 |
Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: