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 XoopsForm 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 XoopsForm, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 31 | class XoopsForm |
||
| 32 | { |
||
| 33 | /** |
||
| 34 | * *#@+ |
||
| 35 | * |
||
| 36 | * @access private |
||
| 37 | */ |
||
| 38 | /** |
||
| 39 | * "action" attribute for the html form |
||
| 40 | * |
||
| 41 | * @var string |
||
| 42 | */ |
||
| 43 | public $_action; |
||
| 44 | |||
| 45 | /** |
||
| 46 | * "method" attribute for the form. |
||
| 47 | * |
||
| 48 | * @var string |
||
| 49 | */ |
||
| 50 | public $_method; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * "name" attribute of the form |
||
| 54 | * |
||
| 55 | * @var string |
||
| 56 | */ |
||
| 57 | public $_name; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * title for the form |
||
| 61 | * |
||
| 62 | * @var string |
||
| 63 | */ |
||
| 64 | public $_title; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * summary for the form (WGAC2 Requirement) |
||
| 68 | * |
||
| 69 | * @var string |
||
| 70 | */ |
||
| 71 | public $_summary = ''; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * array of {@link XoopsFormElement} objects |
||
| 75 | * |
||
| 76 | * @var array |
||
| 77 | */ |
||
| 78 | public $_elements = array(); |
||
| 79 | |||
| 80 | /** |
||
| 81 | * extra information for the <form> tag |
||
| 82 | * |
||
| 83 | * @var array |
||
| 84 | */ |
||
| 85 | public $_extra = array(); |
||
| 86 | |||
| 87 | /** |
||
| 88 | * required elements |
||
| 89 | * |
||
| 90 | * @var array |
||
| 91 | */ |
||
| 92 | public $_required = array(); |
||
| 93 | |||
| 94 | /** |
||
| 95 | * additional serialised object checksum (ERM Analysis - Requirement) |
||
| 96 | * @deprecated |
||
| 97 | * @access private |
||
| 98 | */ |
||
| 99 | public $_objid = 'da39a3ee5e6b4b0d3255bfef95601890afd80709'; |
||
| 100 | |||
| 101 | /** |
||
| 102 | * *#@- |
||
| 103 | */ |
||
| 104 | |||
| 105 | /** |
||
| 106 | * constructor |
||
| 107 | * |
||
| 108 | * @param string $title title of the form |
||
| 109 | * @param string $name "name" attribute for the <form> tag |
||
| 110 | * @param string $action "action" attribute for the <form> tag |
||
| 111 | * @param string $method "method" attribute for the <form> tag |
||
| 112 | * @param bool $addtoken whether to add a security token to the form |
||
| 113 | * @param string $summary |
||
| 114 | */ |
||
| 115 | public function __construct($title, $name, $action, $method = 'post', $addtoken = false, $summary = '') |
||
| 126 | /** |
||
| 127 | * PHP 4 style constructor compatibility shim |
||
| 128 | * @deprecated all callers should be using parent::__construct() |
||
| 129 | */ |
||
| 130 | public function XoopsForm() |
||
| 136 | /** |
||
| 137 | * *#@+ |
||
| 138 | * retrieves object serialisation/identification id (sha1 used) |
||
| 139 | * |
||
| 140 | * each object has serialisation<br> |
||
| 141 | * - legal requirement of enterprise relational management (ERM) |
||
| 142 | * |
||
| 143 | * @deprecated |
||
| 144 | * @access public |
||
| 145 | * @param $object |
||
| 146 | * @param string $hashinfo |
||
| 147 | * @return string |
||
| 148 | */ |
||
| 149 | public function getObjectID($object, $hashinfo = 'sha1') |
||
| 195 | |||
| 196 | /** |
||
| 197 | * @param $value |
||
| 198 | * @param $key |
||
| 199 | * @param $ret |
||
| 200 | * @param string $hashinfo |
||
| 201 | * |
||
| 202 | * @return string |
||
| 203 | */ |
||
| 204 | public function getArrayID($value, $key, $ret, $hashinfo = 'sha1') |
||
| 231 | |||
| 232 | /** |
||
| 233 | * return the summary of the form |
||
| 234 | * |
||
| 235 | * @param bool $encode To sanitizer the text? |
||
| 236 | * @return string |
||
| 237 | */ |
||
| 238 | public function getSummary($encode = false) |
||
| 242 | |||
| 243 | /** |
||
| 244 | * return the title of the form |
||
| 245 | * |
||
| 246 | * @param bool $encode To sanitizer the text? |
||
| 247 | * @return string |
||
| 248 | */ |
||
| 249 | public function getTitle($encode = false) |
||
| 253 | |||
| 254 | /** |
||
| 255 | * get the "name" attribute for the <form> tag |
||
| 256 | * |
||
| 257 | * Deprecated, to be refactored |
||
| 258 | * |
||
| 259 | * @param bool $encode To sanitizer the text? |
||
| 260 | * @return string |
||
| 261 | */ |
||
| 262 | public function getName($encode = true) |
||
| 266 | |||
| 267 | /** |
||
| 268 | * get the "action" attribute for the <form> tag |
||
| 269 | * |
||
| 270 | * @param bool $encode To sanitizer the text? |
||
| 271 | * @return string |
||
| 272 | */ |
||
| 273 | public function getAction($encode = true) |
||
| 278 | |||
| 279 | /** |
||
| 280 | * get the "method" attribute for the <form> tag |
||
| 281 | * |
||
| 282 | * @return string |
||
| 283 | */ |
||
| 284 | public function getMethod() |
||
| 288 | |||
| 289 | /** |
||
| 290 | * Add an element to the form |
||
| 291 | * |
||
| 292 | * @param string|XoopsFormElement $formElement reference to a {@link XoopsFormElement} |
||
| 293 | * @param bool $required is this a "required" element? |
||
| 294 | * |
||
| 295 | */ |
||
| 296 | public function addElement($formElement, $required = false) |
||
| 316 | |||
| 317 | /** |
||
| 318 | * get an array of forms elements |
||
| 319 | * |
||
| 320 | * @param bool $recurse get elements recursively? |
||
| 321 | * |
||
| 322 | * @return array array of {@link XoopsFormElement}s |
||
| 323 | */ |
||
| 324 | View Code Duplication | public function &getElements($recurse = false) |
|
| 349 | |||
| 350 | /** |
||
| 351 | * get an array of "name" attributes of form elements |
||
| 352 | * |
||
| 353 | * @return array array of form element names |
||
| 354 | */ |
||
| 355 | public function getElementNames() |
||
| 366 | |||
| 367 | /** |
||
| 368 | * get a reference to a {@link XoopsFormElement} object by its "name" |
||
| 369 | * |
||
| 370 | * @param string $name "name" attribute assigned to a {@link XoopsFormElement} |
||
| 371 | * @return object reference to a {@link XoopsFormElement}, false if not found |
||
| 372 | */ |
||
| 373 | public function &getElementByName($name) |
||
| 386 | |||
| 387 | /** |
||
| 388 | * Sets the "value" attribute of a form element |
||
| 389 | * |
||
| 390 | * @param string $name the "name" attribute of a form element |
||
| 391 | * @param string $value the "value" attribute of a form element |
||
| 392 | */ |
||
| 393 | public function setElementValue($name, $value) |
||
| 400 | |||
| 401 | /** |
||
| 402 | * Sets the "value" attribute of form elements in a batch |
||
| 403 | * |
||
| 404 | * @param array $values array of name/value pairs to be assigned to form elements |
||
| 405 | */ |
||
| 406 | public function setElementValues($values) |
||
| 420 | |||
| 421 | /** |
||
| 422 | * Gets the "value" attribute of a form element |
||
| 423 | * |
||
| 424 | * @param string $name the "name" attribute of a form element |
||
| 425 | * @param bool $encode To sanitizer the text? |
||
| 426 | * @return string the "value" attribute assigned to a form element, null if not set |
||
| 427 | */ |
||
| 428 | public function getElementValue($name, $encode = false) |
||
| 437 | |||
| 438 | /** |
||
| 439 | * gets the "value" attribute of all form elements |
||
| 440 | * |
||
| 441 | * @param bool $encode To sanitizer the text? |
||
| 442 | * @return array array of name/value pairs assigned to form elements |
||
| 443 | */ |
||
| 444 | public function getElementValues($encode = false) |
||
| 459 | |||
| 460 | /** |
||
| 461 | * set the extra attributes for the <form> tag |
||
| 462 | * |
||
| 463 | * @param string $extra extra attributes for the <form> tag |
||
| 464 | */ |
||
| 465 | public function setExtra($extra) |
||
| 471 | |||
| 472 | /** |
||
| 473 | * set the summary tag for the <form> tag |
||
| 474 | * |
||
| 475 | * @param string $summary |
||
| 476 | */ |
||
| 477 | public function setSummary($summary) |
||
| 483 | |||
| 484 | /** |
||
| 485 | * get the extra attributes for the <form> tag |
||
| 486 | * |
||
| 487 | * @return string |
||
| 488 | */ |
||
| 489 | public function &getExtra() |
||
| 495 | |||
| 496 | /** |
||
| 497 | * make an element "required" |
||
| 498 | * |
||
| 499 | * @param XoopsFormElement $formElement reference to a {@link XoopsFormElement} |
||
| 500 | */ |
||
| 501 | public function setRequired(XoopsFormElement $formElement) |
||
| 505 | |||
| 506 | /** |
||
| 507 | * get an array of "required" form elements |
||
| 508 | * |
||
| 509 | * @return array array of {@link XoopsFormElement}s |
||
| 510 | */ |
||
| 511 | public function &getRequired() |
||
| 515 | |||
| 516 | /** |
||
| 517 | * insert a break in the form |
||
| 518 | * |
||
| 519 | * This method is abstract. It must be overwritten in the child classes. |
||
| 520 | * |
||
| 521 | * @param string $extra extra information for the break |
||
| 522 | * @abstract |
||
| 523 | */ |
||
| 524 | public function insertBreak($extra = null) |
||
| 527 | |||
| 528 | /** |
||
| 529 | * returns renderered form |
||
| 530 | * |
||
| 531 | * This method is abstract. It must be overwritten in the child classes. |
||
| 532 | * |
||
| 533 | * @abstract |
||
| 534 | */ |
||
| 535 | public function render() |
||
| 538 | |||
| 539 | /** |
||
| 540 | * displays rendered form |
||
| 541 | */ |
||
| 542 | public function display() |
||
| 546 | |||
| 547 | /** |
||
| 548 | * Renders the Javascript function needed for client-side for validation |
||
| 549 | * |
||
| 550 | * Form elements that have been declared "required" and not set will prevent the form from being |
||
| 551 | * submitted. Additionally, each element class may provide its own "renderValidationJS" method |
||
| 552 | * that is supposed to return custom validation code for the element. |
||
| 553 | * |
||
| 554 | * The element validation code can assume that the JS "myform" variable points to the form, and must |
||
| 555 | * execute <i>return false</i> if validation fails. |
||
| 556 | * |
||
| 557 | * A basic element validation method may contain something like this: |
||
| 558 | * <code> |
||
| 559 | * function renderValidationJS() { |
||
| 560 | * $name = $this->getName(); |
||
| 561 | * return "if (myform.{$name}.value != 'valid') { " . |
||
| 562 | * "myform.{$name}.focus(); window.alert( '$name is invalid' ); return false;" . |
||
| 563 | * " }"; |
||
| 564 | * } |
||
| 565 | * </code> |
||
| 566 | * |
||
| 567 | * @param boolean $withtags Include the < javascript > tags in the returned string |
||
| 568 | * |
||
| 569 | * @return string |
||
| 570 | */ |
||
| 571 | public function renderValidationJS($withtags = true) |
||
| 592 | |||
| 593 | /** |
||
| 594 | * assign to smarty form template instead of displaying directly |
||
| 595 | * |
||
| 596 | * @param XoopsTpl $tpl reference to a {@link Smarty} object object |
||
| 597 | * @see Smarty |
||
| 598 | */ |
||
| 599 | public function assign(XoopsTpl $tpl) |
||
| 634 | } |
||
| 635 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.