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 UserDefinedForm_Controller 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 UserDefinedForm_Controller, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
373 | class UserDefinedForm_Controller extends Page_Controller |
||
374 | { |
||
375 | |||
376 | private static $finished_anchor = '#uff'; |
||
377 | |||
378 | private static $allowed_actions = array( |
||
379 | 'index', |
||
380 | 'ping', |
||
381 | 'Form', |
||
382 | 'finished' |
||
383 | ); |
||
384 | |||
385 | 6 | public function init() |
|
386 | { |
||
387 | 6 | parent::init(); |
|
388 | |||
389 | 5 | $page = $this->data(); |
|
390 | |||
391 | // load the css |
||
392 | 5 | if (!$page->config()->block_default_userforms_css) { |
|
393 | 5 | Requirements::css(USERFORMS_DIR . '/css/UserForm.css'); |
|
394 | 5 | } |
|
395 | |||
396 | // load the jquery |
||
397 | 5 | if (!$page->config()->block_default_userforms_js) { |
|
398 | 5 | $lang = i18n::get_lang_from_locale(i18n::get_locale()); |
|
399 | 5 | Requirements::javascript(FRAMEWORK_DIR .'/thirdparty/jquery/jquery.js'); |
|
400 | 5 | Requirements::javascript(USERFORMS_DIR . '/thirdparty/jquery-validate/jquery.validate.min.js'); |
|
401 | 5 | Requirements::add_i18n_javascript(USERFORMS_DIR . '/javascript/lang'); |
|
402 | 5 | Requirements::javascript(USERFORMS_DIR . '/javascript/UserForm.js'); |
|
403 | |||
404 | 5 | Requirements::javascript( |
|
405 | 5 | USERFORMS_DIR . "/thirdparty/jquery-validate/localization/messages_{$lang}.min.js" |
|
406 | 5 | ); |
|
407 | 5 | Requirements::javascript( |
|
408 | 5 | USERFORMS_DIR . "/thirdparty/jquery-validate/localization/methods_{$lang}.min.js" |
|
409 | 5 | ); |
|
410 | |||
411 | // Bind a confirmation message when navigating away from a partially completed form. |
||
412 | 5 | if ($page::config()->enable_are_you_sure) { |
|
413 | 5 | Requirements::javascript(USERFORMS_DIR . '/thirdparty/jquery.are-you-sure/jquery.are-you-sure.js'); |
|
414 | 5 | } |
|
415 | 5 | } |
|
416 | 5 | } |
|
417 | |||
418 | /** |
||
419 | * Using $UserDefinedForm in the Content area of the page shows |
||
420 | * where the form should be rendered into. If it does not exist |
||
421 | * then default back to $Form. |
||
422 | * |
||
423 | * @return array |
||
424 | */ |
||
425 | 6 | public function index() |
|
443 | |||
444 | /** |
||
445 | * Keep the session alive for the user. |
||
446 | * |
||
447 | * @return int |
||
448 | */ |
||
449 | public function ping() |
||
453 | |||
454 | /** |
||
455 | * Get the form for the page. Form can be modified by calling {@link updateForm()} |
||
456 | * on a UserDefinedForm extension. |
||
457 | * |
||
458 | * @return Forms |
||
459 | */ |
||
460 | 9 | public function Form() |
|
467 | |||
468 | /** |
||
469 | * Generate the javascript for the conditional field show / hiding logic. |
||
470 | * |
||
471 | * @return void |
||
472 | */ |
||
473 | 9 | public function generateConditionalJavascript() |
|
474 | { |
||
475 | 9 | $default = ""; |
|
476 | 9 | $rules = ""; |
|
477 | |||
478 | 9 | $watch = array(); |
|
479 | |||
480 | 9 | if ($this->Fields()) { |
|
481 | /** @var EditableFormField $field */ |
||
482 | 9 | foreach ($this->Fields() as $field) { |
|
483 | 8 | if ($result = $field->formatDisplayRules()) { |
|
484 | $watch[] = $result; |
||
485 | } |
||
486 | 9 | } |
|
487 | 9 | } |
|
488 | 9 | if ($watch) { |
|
489 | $rules .= $this->buildWatchJS($watch); |
||
490 | } |
||
491 | |||
492 | // Only add customScript if $default or $rules is defined |
||
493 | 9 | if ($rules) { |
|
494 | Requirements::customScript(<<<JS |
||
495 | (function($) { |
||
496 | $(document).ready(function() { |
||
497 | {$rules} |
||
498 | }); |
||
499 | })(jQuery); |
||
500 | JS |
||
501 | , 'UserFormsConditional'); |
||
502 | } |
||
503 | 9 | } |
|
504 | |||
505 | /** |
||
506 | * Process the form that is submitted through the site |
||
507 | * |
||
508 | * {@see UserForm::validate()} for validation step prior to processing |
||
509 | * |
||
510 | * @param array $data |
||
511 | * @param Form $form |
||
512 | * |
||
513 | * @return Redirection |
||
514 | */ |
||
515 | 3 | public function process($data, $form) |
|
516 | { |
||
517 | 2 | $submittedForm = Object::create('SubmittedForm'); |
|
518 | 2 | $submittedForm->SubmittedByID = ($id = Member::currentUserID()) ? $id : 0; |
|
519 | 2 | $submittedForm->ParentID = $this->ID; |
|
520 | |||
521 | // if saving is not disabled save now to generate the ID |
||
522 | 2 | if (!$this->DisableSaveSubmissions) { |
|
523 | 2 | $submittedForm->write(); |
|
524 | 2 | } |
|
525 | |||
526 | 2 | $attachments = array(); |
|
527 | 2 | $submittedFields = new ArrayList(); |
|
528 | |||
529 | 2 | foreach ($this->Fields() as $field) { |
|
530 | 2 | if (!$field->showInReports()) { |
|
531 | 2 | continue; |
|
532 | } |
||
533 | |||
534 | 2 | $submittedField = $field->getSubmittedFormField(); |
|
535 | 2 | $submittedField->ParentID = $submittedForm->ID; |
|
536 | 2 | $submittedField->Name = $field->Name; |
|
537 | 2 | $submittedField->Title = $field->getField('Title'); |
|
538 | |||
539 | // save the value from the data |
||
540 | 2 | if ($field->hasMethod('getValueFromData')) { |
|
541 | $submittedField->Value = $field->getValueFromData($data); |
||
542 | } else { |
||
543 | 2 | if (isset($data[$field->Name])) { |
|
544 | 2 | $submittedField->Value = $data[$field->Name]; |
|
545 | 2 | } |
|
546 | } |
||
547 | |||
548 | 2 | if (!empty($data[$field->Name])) { |
|
549 | 2 | if (in_array("EditableFileField", $field->getClassAncestry())) { |
|
550 | if (!empty($_FILES[$field->Name]['name'])) { |
||
551 | $foldername = $field->getFormField()->getFolderName(); |
||
552 | |||
553 | // create the file from post data |
||
554 | $upload = new Upload(); |
||
555 | $file = new File(); |
||
556 | 1 | $file->ShowInSearch = 0; |
|
557 | try { |
||
558 | $upload->loadIntoFile($_FILES[$field->Name], $file, $foldername); |
||
559 | } catch (ValidationException $e) { |
||
560 | $validationResult = $e->getResult(); |
||
561 | $form->addErrorMessage($field->Name, $validationResult->message(), 'bad'); |
||
562 | Controller::curr()->redirectBack(); |
||
563 | return; |
||
564 | 1 | } |
|
565 | |||
566 | // write file to form field |
||
567 | $submittedField->UploadedFileID = $file->ID; |
||
568 | |||
569 | // attach a file only if lower than 1MB |
||
570 | if ($file->getAbsoluteSize() < 1024*1024*1) { |
||
571 | $attachments[] = $file; |
||
572 | } |
||
573 | } |
||
574 | } |
||
575 | 2 | } |
|
576 | |||
577 | 2 | $submittedField->extend('onPopulationFromField', $field); |
|
578 | |||
579 | 2 | if (!$this->DisableSaveSubmissions) { |
|
580 | 2 | $submittedField->write(); |
|
581 | 2 | } |
|
582 | |||
583 | 2 | $submittedFields->push($submittedField); |
|
584 | 2 | } |
|
585 | |||
586 | $emailData = array( |
||
587 | 2 | "Sender" => Member::currentUser(), |
|
588 | "Fields" => $submittedFields |
||
589 | 2 | ); |
|
590 | |||
591 | 2 | $this->extend('updateEmailData', $emailData, $attachments); |
|
592 | |||
593 | // email users on submit. |
||
594 | 2 | if ($recipients = $this->FilteredEmailRecipients($data, $form)) { |
|
595 | 2 | foreach ($recipients as $recipient) { |
|
596 | 1 | $email = new UserFormRecipientEmail($submittedFields); |
|
597 | 1 | $mergeFields = $this->getMergeFieldsMap($emailData['Fields']); |
|
598 | |||
599 | 1 | if ($attachments) { |
|
600 | foreach ($attachments as $file) { |
||
601 | 1 | if ($file->ID != 0) { |
|
602 | $email->attachFile( |
||
603 | $file->Filename, |
||
604 | $file->Filename, |
||
605 | HTTP::get_mime_type($file->Filename) |
||
606 | ); |
||
607 | } |
||
608 | } |
||
609 | } |
||
610 | |||
611 | 1 | $parsedBody = SSViewer::execute_string($recipient->getEmailBodyContent(), $mergeFields); |
|
612 | |||
613 | 1 | if (!$recipient->SendPlain && $recipient->emailTemplateExists()) { |
|
614 | $email->setTemplate($recipient->EmailTemplate); |
||
615 | } |
||
616 | |||
617 | 1 | $email->populateTemplate($recipient); |
|
618 | 1 | $email->populateTemplate($emailData); |
|
619 | 1 | $email->setFrom($recipient->EmailFrom); |
|
620 | 1 | $email->setBody($parsedBody); |
|
621 | 1 | $email->setTo($recipient->EmailAddress); |
|
622 | 1 | $email->setSubject($recipient->EmailSubject); |
|
623 | |||
624 | 1 | if ($recipient->EmailReplyTo) { |
|
625 | $email->setReplyTo($recipient->EmailReplyTo); |
||
626 | 3 | } |
|
627 | |||
628 | // check to see if they are a dynamic reply to. eg based on a email field a user selected |
||
629 | 1 | View Code Duplication | if ($recipient->SendEmailFromField()) { |
630 | 1 | $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailFromField()->Name); |
|
631 | |||
632 | 1 | if ($submittedFormField && is_string($submittedFormField->Value)) { |
|
633 | $email->setReplyTo($submittedFormField->Value); |
||
634 | } |
||
635 | 1 | } |
|
636 | // check to see if they are a dynamic reciever eg based on a dropdown field a user selected |
||
637 | 1 | View Code Duplication | if ($recipient->SendEmailToField()) { |
638 | 1 | $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailToField()->Name); |
|
639 | |||
640 | 1 | if ($submittedFormField && is_string($submittedFormField->Value)) { |
|
641 | $email->setTo($submittedFormField->Value); |
||
642 | } |
||
643 | 1 | } |
|
644 | |||
645 | // check to see if there is a dynamic subject |
||
646 | 1 | View Code Duplication | if ($recipient->SendEmailSubjectField()) { |
647 | 1 | $submittedFormField = $submittedFields->find('Name', $recipient->SendEmailSubjectField()->Name); |
|
648 | |||
649 | 1 | if ($submittedFormField && trim($submittedFormField->Value)) { |
|
650 | $email->setSubject($submittedFormField->Value); |
||
651 | } |
||
652 | 1 | } |
|
653 | |||
654 | 1 | $this->extend('updateEmail', $email, $recipient, $emailData); |
|
655 | |||
656 | 1 | if ($recipient->SendPlain) { |
|
657 | 1 | $body = strip_tags($recipient->getEmailBodyContent()) . "\n"; |
|
658 | 1 | if (isset($emailData['Fields']) && !$recipient->HideFormData) { |
|
659 | 1 | foreach ($emailData['Fields'] as $Field) { |
|
660 | 1 | $body .= $Field->Title .': '. $Field->Value ." \n"; |
|
661 | 1 | } |
|
662 | 1 | } |
|
663 | |||
664 | 1 | $email->setBody($body); |
|
665 | 1 | $email->sendPlain(); |
|
666 | 1 | } else { |
|
667 | 1 | $email->send(); |
|
668 | } |
||
669 | 3 | } |
|
670 | 2 | } |
|
671 | |||
672 | 2 | $submittedForm->extend('updateAfterProcess'); |
|
673 | |||
674 | 2 | Session::clear("FormInfo.{$form->FormName()}.errors"); |
|
675 | 2 | Session::clear("FormInfo.{$form->FormName()}.data"); |
|
676 | |||
677 | 2 | $referrer = (isset($data['Referrer'])) ? '?referrer=' . urlencode($data['Referrer']) : ""; |
|
678 | |||
679 | // set a session variable from the security ID to stop people accessing |
||
680 | // the finished method directly. |
||
681 | 2 | if (!$this->DisableAuthenicatedFinishAction) { |
|
682 | 2 | if (isset($data['SecurityID'])) { |
|
683 | Session::set('FormProcessed', $data['SecurityID']); |
||
684 | } else { |
||
685 | // if the form has had tokens disabled we still need to set FormProcessed |
||
686 | // to allow us to get through the finshed method |
||
687 | 2 | if (!$this->Form()->getSecurityToken()->isEnabled()) { |
|
688 | 2 | $randNum = rand(1, 1000); |
|
689 | 2 | $randHash = md5($randNum); |
|
690 | 2 | Session::set('FormProcessed', $randHash); |
|
691 | 2 | Session::set('FormProcessedNum', $randNum); |
|
692 | 2 | } |
|
693 | } |
||
694 | 2 | } |
|
695 | |||
696 | 2 | if (!$this->DisableSaveSubmissions) { |
|
697 | 2 | Session::set('userformssubmission'. $this->ID, $submittedForm->ID); |
|
698 | 2 | } |
|
699 | |||
700 | 2 | return $this->redirect($this->Link('finished') . $referrer . $this->config()->finished_anchor); |
|
701 | } |
||
702 | |||
703 | /** |
||
704 | * Allows the use of field values in email body. |
||
705 | * |
||
706 | * @param ArrayList fields |
||
707 | * @return ArrayData |
||
708 | */ |
||
709 | 1 | private function getMergeFieldsMap($fields = array()) |
|
719 | |||
720 | /** |
||
721 | * This action handles rendering the "finished" message, which is |
||
722 | * customizable by editing the ReceivedFormSubmission template. |
||
723 | * |
||
724 | * @return ViewableData |
||
725 | */ |
||
726 | 3 | public function finished() |
|
768 | |||
769 | /** |
||
770 | * Outputs the required JS from the $watch input |
||
771 | * |
||
772 | * @param array $watch |
||
773 | * |
||
774 | * @return string |
||
775 | */ |
||
776 | protected function buildWatchJS($watch) |
||
803 | } |
||
804 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.