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 ClassFormElements 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 ClassFormElements, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 29 | class ClassFormElements extends TDMCreateFile |
||
| 30 | { |
||
| 31 | /* |
||
| 32 | * @public function constructor |
||
| 33 | * @param null |
||
| 34 | */ |
||
| 35 | /** |
||
| 36 | * |
||
| 37 | */ |
||
| 38 | public function __construct() |
||
| 44 | |||
| 45 | /* |
||
| 46 | * @static function &getInstance |
||
| 47 | * @param null |
||
| 48 | */ |
||
| 49 | /** |
||
| 50 | * @return ClassFormElements |
||
| 51 | */ |
||
| 52 | public static function &getInstance() |
||
| 61 | |||
| 62 | /* |
||
| 63 | * @public function initForm |
||
| 64 | * @param string $module |
||
| 65 | * @param string $table |
||
| 66 | */ |
||
| 67 | /** |
||
| 68 | * @param $module |
||
| 69 | * @param $table |
||
| 70 | */ |
||
| 71 | public function initForm($module, $table) |
||
| 76 | |||
| 77 | /* |
||
| 78 | * @private function getXoopsFormText |
||
| 79 | * @param string $language |
||
| 80 | * @param string $fieldName |
||
| 81 | * @param string $required |
||
| 82 | */ |
||
| 83 | /** |
||
| 84 | * @param $language |
||
| 85 | * @param $fieldName |
||
| 86 | * @param $required |
||
| 87 | * |
||
| 88 | * @return string |
||
| 89 | */ |
||
| 90 | private function getXoopsFormText($language, $fieldName, $fieldDefault, $required = 'false') |
||
| 91 | { |
||
| 92 | $ucfFieldName = $this->tdmcfile->getCamelCase($fieldName, true); |
||
| 93 | $ccFieldName = $this->tdmcfile->getCamelCase($fieldName, false, true); |
||
| 94 | if ($fieldDefault != '') { |
||
| 95 | $ret = <<<EOT |
||
| 96 | // Form Text {$ucfFieldName} |
||
| 97 | \${$ccFieldName} = \$this->isNew() ? '{$fieldDefault}' : \$this->getVar('{$fieldName}'); |
||
| 98 | \$form->addElement( new XoopsFormText({$language}, '{$fieldName}', 20, 150, \${$ccFieldName}){$required} );\n |
||
| 99 | EOT; |
||
| 100 | } else { |
||
| 101 | $ret = <<<EOT |
||
| 102 | // Form Text {$ucfFieldName} |
||
| 103 | \$form->addElement( new XoopsFormText({$language}, '{$fieldName}', 50, 255, \$this->getVar('{$fieldName}')){$required} );\n |
||
| 104 | EOT; |
||
| 105 | } |
||
| 106 | |||
| 107 | return $ret; |
||
| 108 | } |
||
| 109 | |||
| 110 | /* |
||
| 111 | * @private function getXoopsFormText |
||
| 112 | * @param string $language |
||
| 113 | * @param string $fieldName |
||
| 114 | * @param string $required |
||
| 115 | */ |
||
| 116 | /** |
||
| 117 | * @param $language |
||
| 118 | * @param $fieldName |
||
| 119 | * @param string $required |
||
| 120 | * |
||
| 121 | * @return string |
||
| 122 | */ |
||
| 123 | private function getXoopsFormTextArea($language, $fieldName, $required = 'false') |
||
| 132 | |||
| 133 | /* |
||
| 134 | * @private function getXoopsFormDhtmlTextArea |
||
| 135 | * @param string $language |
||
| 136 | * @param string $moduleDirname |
||
| 137 | * @param string $fieldName |
||
| 138 | * @param string $required |
||
| 139 | */ |
||
| 140 | /** |
||
| 141 | * @param $language |
||
| 142 | * @param $moduleDirname |
||
| 143 | * @param $fieldName |
||
| 144 | * @param string $required |
||
| 145 | * |
||
| 146 | * @return string |
||
| 147 | */ |
||
| 148 | private function getXoopsFormDhtmlTextArea($language, $moduleDirname, $fieldName, $required = 'false') |
||
| 166 | |||
| 167 | /* |
||
| 168 | * @private function getXoopsFormCheckBox |
||
| 169 | * @param string $language |
||
| 170 | * @param string $fieldName |
||
| 171 | * @param string $required |
||
| 172 | */ |
||
| 173 | /** |
||
| 174 | * @param $language |
||
| 175 | * @param $fieldName |
||
| 176 | * @param string $required |
||
| 177 | * |
||
| 178 | * @return string |
||
| 179 | */ |
||
| 180 | private function getXoopsFormCheckBox($language, $tableSoleName, $fieldName, $fieldElementId, $required = 'false') |
||
| 208 | |||
| 209 | /* |
||
| 210 | * @private function getXoopsFormHidden |
||
| 211 | * @param string $fieldName |
||
| 212 | */ |
||
| 213 | /** |
||
| 214 | * @param $fieldName |
||
| 215 | * |
||
| 216 | * @return string |
||
| 217 | */ |
||
| 218 | private function getXoopsFormHidden($fieldName) |
||
| 227 | |||
| 228 | /* |
||
| 229 | * @private function getXoopsFormImageList |
||
| 230 | * @param string $language |
||
| 231 | * @param string $moduleDirname |
||
| 232 | * @param string $tableName |
||
| 233 | * @param string $fieldName |
||
| 234 | * @param string $required |
||
| 235 | */ |
||
| 236 | /** |
||
| 237 | * @param $language |
||
| 238 | * @param $moduleDirname |
||
| 239 | * @param $tableName |
||
| 240 | * @param $tableSoleName |
||
| 241 | * @param $fieldName |
||
| 242 | * @param string $required |
||
| 243 | * |
||
| 244 | * @return string |
||
| 245 | */ |
||
| 246 | private function getXoopsFormImageList($language, $moduleDirname, $tableName, $tableSoleName, $fieldName, $required = 'false') |
||
| 278 | |||
| 279 | /* |
||
| 280 | * @private function getXoopsFormSelectFile |
||
| 281 | * @param string $language |
||
| 282 | * @param string $moduleDirname |
||
| 283 | * @param string $fieldName |
||
| 284 | * @param string $fieldDefault |
||
| 285 | * @param string $fieldElement |
||
| 286 | * @param string $required |
||
| 287 | */ |
||
| 288 | /** |
||
| 289 | * @param $language |
||
| 290 | * @param $moduleDirname |
||
| 291 | * @param $fieldName |
||
| 292 | * @param $fieldElement |
||
| 293 | * @param string $required |
||
| 294 | * |
||
| 295 | * @return string |
||
| 296 | */ |
||
| 297 | private function getXoopsFormSelectFile($language, $moduleDirname, $fieldName, $fieldElement, $required = 'false') |
||
| 298 | { |
||
| 299 | $ucfFieldName = $this->tdmcfile->getCamelCase($fieldName, true); |
||
| 300 | $ccFieldName = $this->tdmcfile->getCamelCase($fieldName, false, true); |
||
| 301 | $ret = <<<EOT |
||
| 302 | // Image Select or Upload |
||
| 303 | if ( \$this->{$moduleDirname}->getConfig('useshots') ) { |
||
| 304 | \${$ccFieldName} = \$this->getVar('{$fieldName}'); |
||
| 305 | \$uploadDirectory = '/uploads/{$moduleDirname}/images/shots'; |
||
| 306 | \${$moduleDirname}ShotImage = \${$fieldName} ? \${$fieldName} : 'blank.gif'; |
||
| 307 | // |
||
| 308 | \$imageTray = new XoopsFormElementTray({$language}FORM_IMAGE,'<br />'); |
||
| 309 | \$imageSelect = new XoopsFormSelect(sprintf({$language}_FORM_PATH, \$uploadDirectory ), 'selected_image',\${$moduleDirname}ShotImage); |
||
| 310 | \$imageArray = XoopsLists::getImgListAsArray( XOOPS_ROOT_PATH . \$uploadDirectory ); |
||
| 311 | foreach( \$imageArray as \$image ) { |
||
| 312 | \$imageSelect->addOption("{\$image}", \$image); |
||
| 313 | } |
||
| 314 | \$imageSelect->setExtra( "onchange='showImgSelected(\"image3\", \"selected_image\", \"" . \$uploadDirectory . "\", \"\", \"" . XOOPS_URL . "\")'" ); |
||
| 315 | \$imageTray->addElement(\$imageSelect,false); |
||
| 316 | \$imageTray -> addElement( new XoopsFormLabel( '', "<br /><img src='" . XOOPS_URL . "/" . \$uploadDirectory . "/" . \${$moduleDirname}ShotImage . "' name='image3' id='image3' alt='' />" ) ); |
||
| 317 | \$fileSelectTray= new XoopsFormElementTray('','<br />'); |
||
| 318 | //if (\$permissionUpload == true) { |
||
| 319 | \$fileSelectTray->addElement(new XoopsFormFile({$language}_FORM_UPLOAD , 'attachedimage', \$this->{$moduleDirname}->getConfig('maxuploadsize')){$required}); |
||
| 320 | //} |
||
| 321 | \$imageTray->addElement(\$fileSelectTray); |
||
| 322 | \$form->addElement(\$imageTray); |
||
| 323 | }\n |
||
| 324 | EOT; |
||
| 325 | |||
| 326 | return $ret; |
||
| 327 | } |
||
| 328 | |||
| 329 | /* |
||
| 330 | * @private function getXoopsFormUrlFile |
||
| 331 | * @param string $language |
||
| 332 | * @param string $moduleDirname |
||
| 333 | * @param string $fieldName |
||
| 334 | * @param string $fieldDefault |
||
| 335 | * @param string $fieldElement |
||
| 336 | * @param string $required |
||
| 337 | */ |
||
| 338 | /** |
||
| 339 | * @param $language |
||
| 340 | * @param $moduleDirname |
||
| 341 | * @param $fieldName |
||
| 342 | * @param $fieldDefault |
||
| 343 | * @param $fieldElement |
||
| 344 | * @param string $required |
||
| 345 | * |
||
| 346 | * @return string |
||
| 347 | */ |
||
| 348 | private function getXoopsFormUrlFile($language, $moduleDirname, $fieldName, $fieldDefault, $fieldElement, $required = 'false') |
||
| 362 | |||
| 363 | /* |
||
| 364 | * @private function getXoopsFormUploadImage |
||
| 365 | * @param string $language |
||
| 366 | * @param string $moduleDirname |
||
| 367 | * @param string $tableName |
||
| 368 | * @param string $required |
||
| 369 | */ |
||
| 370 | /** |
||
| 371 | * @param $language |
||
| 372 | * @param $moduleDirname |
||
| 373 | * @param $tableName |
||
| 374 | * @param string $required |
||
| 375 | * |
||
| 376 | * @return string |
||
| 377 | */ |
||
| 378 | private function getXoopsFormUploadImage($language, $moduleDirname, $tableName, $tableSoleName, $fieldName, $required = 'false') |
||
| 410 | |||
| 411 | /* |
||
| 412 | * @private function getXoopsFormUploadFile |
||
| 413 | * @param string $language |
||
| 414 | * @param string $moduleDirname |
||
| 415 | * @param string $tableName |
||
| 416 | * @param string $fieldName |
||
| 417 | * @param string $required |
||
| 418 | */ |
||
| 419 | /** |
||
| 420 | * @param $language |
||
| 421 | * @param $moduleDirname |
||
| 422 | * @param $tableName |
||
| 423 | * @param $fieldName |
||
| 424 | * @param string $required |
||
| 425 | * |
||
| 426 | * @return string |
||
| 427 | */ |
||
| 428 | private function getXoopsFormUploadFile($language, $moduleDirname, $tableName, $fieldName, $required = 'false') |
||
| 438 | |||
| 439 | /* |
||
| 440 | * @private function getXoopsFormColorPicker |
||
| 441 | * @param string $language |
||
| 442 | * @param string $fieldName |
||
| 443 | * @param string $required |
||
| 444 | */ |
||
| 445 | /** |
||
| 446 | * @param $language |
||
| 447 | * @param $moduleDirname |
||
| 448 | * @param $fieldName |
||
| 449 | * @param string $required |
||
| 450 | * |
||
| 451 | * @return string |
||
| 452 | */ |
||
| 453 | private function getXoopsFormColorPicker($language, $moduleDirname, $fieldName, $required = 'false') |
||
| 462 | |||
| 463 | /* |
||
| 464 | * @private function getXoopsFormSelectBox |
||
| 465 | * @param string $language |
||
| 466 | * @param string $tableName |
||
| 467 | * @param string $fieldName |
||
| 468 | * @param string $required |
||
| 469 | */ |
||
| 470 | /** |
||
| 471 | * @param $language |
||
| 472 | * @param $moduleDirname |
||
| 473 | * @param $tableName |
||
| 474 | * @param $fieldName |
||
| 475 | * @param $required |
||
| 476 | * |
||
| 477 | * @return string |
||
| 478 | */ |
||
| 479 | View Code Duplication | private function getXoopsFormSelectBox($language, $moduleDirname, $tableName, $fieldName, $required = 'false') |
|
| 495 | |||
| 496 | /* |
||
| 497 | * @private function getXoopsFormSelectUser |
||
| 498 | * @param string $language |
||
| 499 | * @param string $fieldName |
||
| 500 | * @param string $required |
||
| 501 | */ |
||
| 502 | /** |
||
| 503 | * @param $language |
||
| 504 | * @param $fieldName |
||
| 505 | * @param string $required |
||
| 506 | * |
||
| 507 | * @return string |
||
| 508 | */ |
||
| 509 | private function getXoopsFormSelectUser($language, $fieldName, $required = 'false') |
||
| 518 | |||
| 519 | /* |
||
| 520 | * @private function getXoopsFormRadioYN |
||
| 521 | * @param string $language |
||
| 522 | * @param string $fieldName |
||
| 523 | * @param string $required |
||
| 524 | */ |
||
| 525 | /** |
||
| 526 | * @param $language |
||
| 527 | * @param $fieldName |
||
| 528 | * @param string $required |
||
| 529 | * |
||
| 530 | * @return string |
||
| 531 | */ |
||
| 532 | private function getXoopsFormRadioYN($language, $fieldName, $required = 'false') |
||
| 543 | |||
| 544 | /* |
||
| 545 | * @private function getXoopsFormTextDateSelect |
||
| 546 | * @param string $language |
||
| 547 | * @param string $fieldName |
||
| 548 | * @param string $required |
||
| 549 | */ |
||
| 550 | /** |
||
| 551 | * @param $language |
||
| 552 | * @param $moduleDirname |
||
| 553 | * @param $fieldName |
||
| 554 | * @param string $required |
||
| 555 | * |
||
| 556 | * @return string |
||
| 557 | */ |
||
| 558 | private function getXoopsFormTextDateSelect($language, $moduleDirname, $fieldName, $required = 'false') |
||
| 567 | |||
| 568 | /* |
||
| 569 | * @private function getXoopsFormTable |
||
| 570 | * @param string $language |
||
| 571 | * @param string $moduleDirname |
||
| 572 | * @param string $table |
||
| 573 | * @param string $fields |
||
| 574 | * @param string $required |
||
| 575 | */ |
||
| 576 | /** |
||
| 577 | * @param $language |
||
| 578 | * @param $moduleDirname |
||
| 579 | * @param $tableName |
||
| 580 | * @param $fieldName |
||
| 581 | * @param $fieldElement |
||
| 582 | * @param string $required |
||
| 583 | * |
||
| 584 | * @return string |
||
| 585 | */ |
||
| 586 | private function getXoopsFormTable($language, $moduleDirname, $tableName, $fieldName, $fieldElement, $required = 'false') |
||
| 604 | |||
| 605 | /* |
||
| 606 | * @private function getXoopsFormTopic |
||
| 607 | * @param string $language |
||
| 608 | * @param string $moduleDirname |
||
| 609 | * @param string $table |
||
| 610 | * @param string $fields |
||
| 611 | * @param string $required |
||
| 612 | */ |
||
| 613 | /** |
||
| 614 | * @param $language |
||
| 615 | * @param $moduleDirname |
||
| 616 | * @param $table |
||
| 617 | * @param $fields |
||
| 618 | * @param string $required |
||
| 619 | * |
||
| 620 | * @return string |
||
| 621 | */ |
||
| 622 | private function getXoopsFormTopic($language, $moduleDirname, $topicTableName, $fieldId, $fieldPid, $fieldMain, $required = 'false') |
||
| 644 | |||
| 645 | /* |
||
| 646 | * @private function getXoopsFormTag |
||
| 647 | * @param string $moduleDirname |
||
| 648 | * @param string $fieldId |
||
| 649 | * @param string $required |
||
| 650 | */ |
||
| 651 | /** |
||
| 652 | * @param $moduleDirname |
||
| 653 | * @param $fieldId |
||
| 654 | * @param string $required |
||
| 655 | * |
||
| 656 | * @return string |
||
| 657 | */ |
||
| 658 | private function getXoopsFormTag($moduleDirname, $fieldId, $required = 'false') |
||
| 672 | |||
| 673 | /* |
||
| 674 | * @public function renderElements |
||
| 675 | * @param null |
||
| 676 | */ |
||
| 677 | /** |
||
| 678 | * @return string |
||
| 679 | */ |
||
| 680 | public function renderElements() |
||
| 807 | } |
||
| 808 |
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.