symphonycms /
symphony-2
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 SectionEvent 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 SectionEvent, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 14 | abstract class SectionEvent extends Event |
||
| 15 | { |
||
| 16 | /** |
||
| 17 | * An associative array of results from the filters that have run |
||
| 18 | * on this event. |
||
| 19 | * |
||
| 20 | * @var array |
||
| 21 | */ |
||
| 22 | public $filter_results = array(); |
||
| 23 | |||
| 24 | /** |
||
| 25 | * An associative array of errors from the filters that have run |
||
| 26 | * on this event. |
||
| 27 | * |
||
| 28 | * @var array |
||
| 29 | */ |
||
| 30 | public $filter_errors = array(); |
||
| 31 | |||
| 32 | /** |
||
| 33 | * Helper method to determine if a field is missing, or if the data |
||
| 34 | * provided was invalid. Used in conjunction with `array_reduce`. |
||
| 35 | * |
||
| 36 | * @param array $a , |
||
| 37 | * @param array $b |
||
| 38 | * @return string |
||
| 39 | * 'missing' or 'invalid' |
||
| 40 | */ |
||
| 41 | public function __reduceType($a, $b) |
||
| 49 | |||
| 50 | /** |
||
| 51 | * This function will process the core Filters, Admin Only and Expect |
||
| 52 | * Multiple, before invoking the `__doit` function, which actually |
||
| 53 | * processes the Event. Once the Event has executed, this function will |
||
| 54 | * determine if the user should be redirected to a URL, or to just return |
||
| 55 | * the XML. |
||
| 56 | * |
||
| 57 | * @throws Exception |
||
| 58 | * @return XMLElement|void |
||
| 59 | * If `$_REQUEST{'redirect']` is set, and the Event executed successfully, |
||
| 60 | * the user will be redirected to the given location. If `$_REQUEST['redirect']` |
||
| 61 | * is not set, or the Event encountered errors, an XMLElement of the Event |
||
| 62 | * result will be returned. |
||
| 63 | */ |
||
| 64 | public function execute() |
||
| 65 | { |
||
| 66 | if (!isset($this->eParamFILTERS) || !is_array($this->eParamFILTERS)) { |
||
| 67 | $this->eParamFILTERS = array(); |
||
|
|
|||
| 68 | } |
||
| 69 | |||
| 70 | $result = new XMLElement($this->ROOTELEMENT); |
||
| 71 | |||
| 72 | if (in_array('admin-only', $this->eParamFILTERS) && !Symphony::Engine()->isLoggedIn()) { |
||
| 73 | $result->setAttribute('result', 'error'); |
||
| 74 | $result->appendChild(new XMLElement('message', __('Entry encountered errors when saving.'), array( |
||
| 75 | 'message-id' => EventMessages::ENTRY_ERRORS |
||
| 76 | ))); |
||
| 77 | $result->appendChild(self::buildFilterElement('admin-only', 'failed')); |
||
| 78 | |||
| 79 | return $result; |
||
| 80 | } |
||
| 81 | |||
| 82 | $entry_id = $position = $fields = null; |
||
| 83 | $post = General::getPostData(); |
||
| 84 | $success = true; |
||
| 85 | if (!is_array($post['fields'])) { |
||
| 86 | $post['fields'] = array(); |
||
| 87 | } |
||
| 88 | |||
| 89 | if (in_array('expect-multiple', $this->eParamFILTERS)) { |
||
| 90 | foreach ($post['fields'] as $position => $fields) { |
||
| 91 | if (isset($post['id'][$position]) && is_numeric($post['id'][$position])) { |
||
| 92 | $entry_id = $post['id'][$position]; |
||
| 93 | } else { |
||
| 94 | $entry_id = null; |
||
| 95 | } |
||
| 96 | |||
| 97 | $entry = new XMLElement('entry', null, array('position' => $position)); |
||
| 98 | |||
| 99 | // Reset errors for each entry execution |
||
| 100 | $this->filter_results = $this->filter_errors = array(); |
||
| 101 | |||
| 102 | // Ensure that we are always dealing with an array. |
||
| 103 | if (!is_array($fields)) { |
||
| 104 | $fields = array(); |
||
| 105 | } |
||
| 106 | |||
| 107 | // Execute the event for this entry |
||
| 108 | if (!$this->__doit($fields, $entry, $position, $entry_id)) { |
||
| 109 | $success = false; |
||
| 110 | } |
||
| 111 | |||
| 112 | $result->appendChild($entry); |
||
| 113 | } |
||
| 114 | } else { |
||
| 115 | $fields = $post['fields']; |
||
| 116 | |||
| 117 | if (isset($post['id']) && is_numeric($post['id'])) { |
||
| 118 | $entry_id = $post['id']; |
||
| 119 | } |
||
| 120 | |||
| 121 | $success = $this->__doit($fields, $result, null, $entry_id); |
||
| 122 | } |
||
| 123 | |||
| 124 | if ($success && isset($_REQUEST['redirect'])) { |
||
| 125 | redirect($_REQUEST['redirect']); |
||
| 126 | } |
||
| 127 | |||
| 128 | return $result; |
||
| 129 | } |
||
| 130 | |||
| 131 | /** |
||
| 132 | * This method will construct XML that represents the result of |
||
| 133 | * an Event filter. |
||
| 134 | * |
||
| 135 | * @param string $name |
||
| 136 | * The name of the filter |
||
| 137 | * @param string $status |
||
| 138 | * The status of the filter, either passed or failed. |
||
| 139 | * @param XMLElement|string $message |
||
| 140 | * Optionally, an XMLElement or string to be appended to this |
||
| 141 | * `<filter>` element. XMLElement allows for more complex return |
||
| 142 | * types. |
||
| 143 | * @param array $attributes |
||
| 144 | * An associative array of additional attributes to add to this |
||
| 145 | * `<filter>` element |
||
| 146 | * @return XMLElement |
||
| 147 | */ |
||
| 148 | public static function buildFilterElement($name, $status, $message = null, array $attributes = null) |
||
| 163 | |||
| 164 | /** |
||
| 165 | * This function does the bulk of processing the Event, from running the delegates |
||
| 166 | * to validating the data and eventually saving the data into Symphony. The result |
||
| 167 | * of the Event is returned via the `$result` parameter. |
||
| 168 | * |
||
| 169 | * @param array $fields |
||
| 170 | * An array of $_POST data, to process and add/edit an entry. |
||
| 171 | * @param XMLElement $result |
||
| 172 | * The XMLElement contains the result of the Event, it is passed by |
||
| 173 | * reference. |
||
| 174 | * @param integer $position |
||
| 175 | * When the Expect Multiple filter is added, this event should expect |
||
| 176 | * to deal with adding (or editing) multiple entries at once. |
||
| 177 | * @param integer $entry_id |
||
| 178 | * If this Event is editing an existing entry, that Entry ID will |
||
| 179 | * be passed to this function. |
||
| 180 | * @throws Exception |
||
| 181 | * @return XMLElement |
||
| 182 | * The result of the Event |
||
| 183 | */ |
||
| 184 | public function __doit(array $fields = array(), XMLElement &$result, $position = null, $entry_id = null) |
||
| 305 | |||
| 306 | /** |
||
| 307 | * Processes all extensions attached to the `EventPreSaveFilter` delegate |
||
| 308 | * |
||
| 309 | * @uses EventPreSaveFilter |
||
| 310 | * |
||
| 311 | * @param XMLElement $result |
||
| 312 | * @param array $fields |
||
| 313 | * @param XMLElement $post_values |
||
| 314 | * @param integer $entry_id |
||
| 315 | * @return boolean |
||
| 316 | */ |
||
| 317 | protected function processPreSaveFilters( |
||
| 388 | |||
| 389 | /** |
||
| 390 | * Appends errors generated from fields during the execution of an Event |
||
| 391 | * |
||
| 392 | * @param XMLElement $result |
||
| 393 | * @param array $fields |
||
| 394 | * @param array $errors |
||
| 395 | * @param object $post_values |
||
| 396 | * @throws Exception |
||
| 397 | * @return XMLElement |
||
| 398 | */ |
||
| 399 | public static function appendErrors(XMLElement $result, array $fields, $errors, $post_values) |
||
| 438 | |||
| 439 | /** |
||
| 440 | * Given a Field instance, the type of error, and the message, this function |
||
| 441 | * creates an XMLElement node so that it can be added to the `?debug` for the |
||
| 442 | * Event |
||
| 443 | * |
||
| 444 | * @since Symphony 2.5.0 |
||
| 445 | * @param Field $field |
||
| 446 | * @param string $type |
||
| 447 | * At the moment 'missing' or 'invalid' accepted |
||
| 448 | * @param string $message |
||
| 449 | * @return XMLElement |
||
| 450 | */ |
||
| 451 | public static function createError(Field $field, $type, $message = null) |
||
| 462 | |||
| 463 | /** |
||
| 464 | * This function handles the Send Mail filter which will send an email |
||
| 465 | * to each specified recipient informing them that an Entry has been |
||
| 466 | * created. |
||
| 467 | * |
||
| 468 | * @param XMLElement $result |
||
| 469 | * The XMLElement of the XML that is going to be returned as part |
||
| 470 | * of this event to the page. |
||
| 471 | * @param array $send_email |
||
| 472 | * Associative array of `send-mail` parameters.* Associative array of `send-mail` parameters. |
||
| 473 | * @param array $fields |
||
| 474 | * Array of post data to extract the values from |
||
| 475 | * @param Section $section |
||
| 476 | * This current Entry that has just been updated or created |
||
| 477 | * @param Entry $entry |
||
| 478 | * @throws Exception |
||
| 479 | * @return XMLElement |
||
| 480 | * The modified `$result` with the results of the filter. |
||
| 481 | */ |
||
| 482 | public function processSendMailFilter( |
||
| 601 | |||
| 602 | /** |
||
| 603 | * This function searches the `$haystack` for the given `$needle`, |
||
| 604 | * where the needle is a string representation of where the desired |
||
| 605 | * value exists in the `$haystack` array. For example `fields[name]` |
||
| 606 | * would look in the `$haystack` for the key of `fields` that has the |
||
| 607 | * key `name` and return the value. |
||
| 608 | * |
||
| 609 | * @param string $needle |
||
| 610 | * The needle, ie. `fields[name]`. |
||
| 611 | * @param array $haystack |
||
| 612 | * Associative array to find the needle, ie. |
||
| 613 | * `array('fields' => array( |
||
| 614 | * 'name' => 'Bob', |
||
| 615 | * 'age' => '10' |
||
| 616 | * ))` |
||
| 617 | * @param string $default |
||
| 618 | * If the `$needle` is not found, return this value. Defaults to null. |
||
| 619 | * @param boolean $discard_field_name |
||
| 620 | * When matches are found in the `$haystack`, they are added to results |
||
| 621 | * array. This parameter defines if this should be an associative array |
||
| 622 | * or just an array of the matches. Used in conjunction with `$collapse` |
||
| 623 | * @param boolean $collapse |
||
| 624 | * If multiple values are found, this will cause them to be reduced |
||
| 625 | * to single string with ' ' as the separator. Defaults to true. |
||
| 626 | * @return string|array |
||
| 627 | */ |
||
| 628 | public static function replaceFieldToken( |
||
| 661 | |||
| 662 | /** |
||
| 663 | * Processes all extensions attached to the `EventPostSaveFilter` delegate |
||
| 664 | * |
||
| 665 | * @uses EventPostSaveFilter |
||
| 666 | * |
||
| 667 | * @param XMLElement $result |
||
| 668 | * @param array $fields |
||
| 669 | * @param Entry $entry |
||
| 670 | * @return XMLElement |
||
| 671 | */ |
||
| 672 | View Code Duplication | protected function processPostSaveFilters(XMLElement $result, array $fields, Entry $entry = null) |
|
| 716 | |||
| 717 | /** |
||
| 718 | * Processes all extensions attached to the `EventFinalSaveFilter` delegate |
||
| 719 | * |
||
| 720 | * @uses EventFinalSaveFilter |
||
| 721 | * |
||
| 722 | * @param XMLElement $result |
||
| 723 | * @param array $fields |
||
| 724 | * @param Entry $entry |
||
| 725 | * @return XMLElement |
||
| 726 | */ |
||
| 727 | View Code Duplication | protected function processFinalSaveFilters(XMLElement $result, array $fields, Entry $entry = null) |
|
| 778 | } |
||
| 779 | |||
| 807 |
Loading history...
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: