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 BaseSmartObject 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 BaseSmartObject, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace XoopsModules\Smartobject; |
||
| 57 | class BaseSmartObject extends \XoopsObject |
||
| 58 | { |
||
| 59 | public $_image_path; |
||
| 60 | public $_image_url; |
||
| 61 | |||
| 62 | public $seoEnabled = false; |
||
| 63 | public $titleField; |
||
| 64 | public $summaryField = false; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * Reference to the handler managing this object |
||
| 68 | * |
||
| 69 | * @var SmartPersistableObjectHandler reference to {@link SmartPersistableObjectHandler} |
||
| 70 | */ |
||
| 71 | public $handler; |
||
| 72 | |||
| 73 | /** |
||
| 74 | * References to control objects, managing the form fields of this object |
||
| 75 | */ |
||
| 76 | public $controls = []; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * SmartObject constructor. |
||
| 80 | * @param $handler |
||
| 81 | */ |
||
| 82 | public function __construct($handler) |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Checks if the user has a specific access on this object |
||
| 89 | * |
||
| 90 | * @param $perm_name |
||
| 91 | * @return bool: TRUE if user has access, false if not |
||
| 92 | * @internal param string $gperm_name name of the permission to test |
||
| 93 | */ |
||
| 94 | public function accessGranted($perm_name) |
||
| 100 | |||
| 101 | /** |
||
| 102 | * @param $section_name |
||
| 103 | * @param bool $value |
||
| 104 | * @param bool $hide |
||
| 105 | */ |
||
| 106 | public function addFormSection($section_name, $value = false, $hide = false) |
||
| 111 | |||
| 112 | /** |
||
| 113 | * @param $section_name |
||
| 114 | */ |
||
| 115 | public function closeSection($section_name) |
||
| 119 | |||
| 120 | /** |
||
| 121 | * |
||
| 122 | * @param string $key key of this field. This needs to be the name of the field in the related database table |
||
| 123 | * @param int $data_type set to one of XOBJ_DTYPE_XXX constants (set to XOBJ_DTYPE_OTHER if no data type ckecking nor text sanitizing is required) |
||
| 124 | * @param mixed $value default value of this variable |
||
| 125 | * @param bool $required set to TRUE if this variable needs to have a value set before storing the object in the table |
||
| 126 | * @param int $maxlength maximum length of this variable, for XOBJ_DTYPE_TXTBOX type only |
||
| 127 | * @param string $options does this data have any select options? |
||
| 128 | * @param bool $multilingual is this field needs to support multilingual features (NOT YET IMPLEMENTED...) |
||
| 129 | * @param string $form_caption caption of this variable in a {@link SmartobjectForm} and title of a column in a {@link SmartObjectTable} |
||
| 130 | * @param string $form_dsc description of this variable in a {@link SmartobjectForm} |
||
| 131 | * @param bool $sortby set to TRUE to make this field used to sort objects in SmartObjectTable |
||
| 132 | * @param bool $persistent set to FALSE if this field is not to be saved in the database |
||
| 133 | * @param bool $displayOnForm |
||
| 134 | */ |
||
| 135 | public function initVar( |
||
| 178 | |||
| 179 | /** |
||
| 180 | * @param $key |
||
| 181 | * @param $data_type |
||
| 182 | * @param bool $itemName |
||
| 183 | * @param string $form_caption |
||
| 184 | * @param bool $sortby |
||
| 185 | * @param string $value |
||
| 186 | * @param bool $displayOnForm |
||
| 187 | * @param bool $required |
||
| 188 | */ |
||
| 189 | View Code Duplication | public function initNonPersistableVar( |
|
| 202 | |||
| 203 | /** |
||
| 204 | * Quickly initiate a var |
||
| 205 | * |
||
| 206 | * Since many vars do have the same config, let's use this method with some of these configuration as a convention ;-) |
||
| 207 | * |
||
| 208 | * - $maxlength = 0 unless $data_type is a TEXTBOX, then $maxlength will be 255 |
||
| 209 | * - all other vars are NULL or '' depending of the parameter |
||
| 210 | * |
||
| 211 | * @param string $key key of this field. This needs to be the name of the field in the related database table |
||
| 212 | * @param int $data_type set to one of XOBJ_DTYPE_XXX constants (set to XOBJ_DTYPE_OTHER if no data type ckecking nor text sanitizing is required) |
||
| 213 | * @param bool $required set to TRUE if this variable needs to have a value set before storing the object in the table |
||
| 214 | * @param string $form_caption caption of this variable in a {@link SmartobjectForm} and title of a column in a {@link SmartObjectTable} |
||
| 215 | * @param string $form_dsc description of this variable in a {@link SmartobjectForm} |
||
| 216 | * @param mixed $value default value of this variable |
||
| 217 | */ |
||
| 218 | View Code Duplication | public function quickInitVar( |
|
| 229 | |||
| 230 | /** |
||
| 231 | * @param $varname |
||
| 232 | * @param bool $displayOnForm |
||
| 233 | * @param string $default |
||
| 234 | */ |
||
| 235 | public function initCommonVar($varname, $displayOnForm = true, $default = 'notdefined') |
||
| 316 | |||
| 317 | /** |
||
| 318 | * Set control information for an instance variable |
||
| 319 | * |
||
| 320 | * The $options parameter can be a string or an array. Using a string |
||
| 321 | * is the quickest way: |
||
| 322 | * |
||
| 323 | * $this->setControl('date', 'date_time'); |
||
| 324 | * |
||
| 325 | * This will create a date and time selectbox for the 'date' var on the |
||
| 326 | * form to edit or create this item. |
||
| 327 | * |
||
| 328 | * Here are the currently supported controls: |
||
| 329 | * |
||
| 330 | * - color |
||
| 331 | * - country |
||
| 332 | * - date_time |
||
| 333 | * - date |
||
| 334 | |||
| 335 | * - group |
||
| 336 | * - group_multi |
||
| 337 | * - image |
||
| 338 | * - imageupload |
||
| 339 | * - label |
||
| 340 | * - language |
||
| 341 | * - parentcategory |
||
| 342 | * - password |
||
| 343 | * - select_multi |
||
| 344 | * - select |
||
| 345 | * - text |
||
| 346 | * - textarea |
||
| 347 | * - theme |
||
| 348 | * - theme_multi |
||
| 349 | * - timezone |
||
| 350 | * - user |
||
| 351 | * - user_multi |
||
| 352 | * - yesno |
||
| 353 | * |
||
| 354 | * Now, using an array as $options, you can customize what information to |
||
| 355 | * use in the control. For example, if one needs to display a select box for |
||
| 356 | * the user to choose the status of an item. We only need to tell SmartObject |
||
| 357 | * what method to execute within what handler to retreive the options of the |
||
| 358 | * selectbox. |
||
| 359 | * |
||
| 360 | * $this->setControl('status', array('name' => false, |
||
| 361 | * 'itemHandler' => 'item', |
||
| 362 | * 'method' => 'getStatus', |
||
| 363 | * 'module' => 'smartshop')); |
||
| 364 | * |
||
| 365 | * In this example, the array elements are the following: |
||
| 366 | * - name: false, as we don't need to set a special control here. |
||
| 367 | * we will use the default control related to the object type (defined in initVar) |
||
| 368 | * - itemHandler: name of the object for which we will use the handler |
||
| 369 | * - method: name of the method of this handler that we will execute |
||
| 370 | * - module: name of the module from wich the handler is |
||
| 371 | * |
||
| 372 | * So in this example, SmartObject will create a selectbox for the variable 'status' and it will |
||
| 373 | * populate this selectbox with the result from SmartshopItemHandler::getStatus() |
||
| 374 | * |
||
| 375 | * Another example of the use of $options as an array is for TextArea: |
||
| 376 | * |
||
| 377 | * $this->setControl('body', array('name' => 'textarea', |
||
| 378 | * 'form_editor' => 'default')); |
||
| 379 | * |
||
| 380 | * In this example, SmartObject will create a TextArea for the variable 'body'. And it will use |
||
| 381 | * the 'default' editor, providing it is defined in the module |
||
| 382 | * preferences: $xoopsModuleConfig['default_editor'] |
||
| 383 | * |
||
| 384 | * Of course, you can force the use of a specific editor: |
||
| 385 | * |
||
| 386 | * $this->setControl('body', array('name' => 'textarea', |
||
| 387 | * 'form_editor' => 'koivi')); |
||
| 388 | * |
||
| 389 | * Here is a list of supported editor: |
||
| 390 | * - tiny: TinyEditor |
||
| 391 | * - dhtmltextarea: XOOPS DHTML Area |
||
| 392 | * - fckeditor: FCKEditor |
||
| 393 | * - inbetween: InBetween |
||
| 394 | * - koivi: Koivi |
||
| 395 | * - spaw: Spaw WYSIWYG Editor |
||
| 396 | * - htmlarea: HTMLArea |
||
| 397 | * - textarea: basic textarea with no options |
||
| 398 | * |
||
| 399 | * @param string $var name of the variable for which we want to set a control |
||
| 400 | * @param array $options |
||
| 401 | */ |
||
| 402 | public function setControl($var, $options = []) |
||
| 412 | |||
| 413 | /** |
||
| 414 | * Get control information for an instance variable |
||
| 415 | * |
||
| 416 | * @param string $var |
||
| 417 | * @return bool|mixed |
||
| 418 | */ |
||
| 419 | public function getControl($var) |
||
| 423 | |||
| 424 | /** |
||
| 425 | * Create the form for this object |
||
| 426 | * |
||
| 427 | * @param $form_caption |
||
| 428 | * @param $form_name |
||
| 429 | * @param bool $form_action |
||
| 430 | * @param string $submit_button_caption |
||
| 431 | * @param bool $cancel_js_action |
||
| 432 | * @param bool $captcha |
||
| 433 | * @return \XoopsModules\Smartobject\Form\SmartobjectForm <a href='psi_element://SmartobjectForm'>SmartobjectForm</a> object for this object |
||
| 434 | * object for this object |
||
| 435 | * @see SmartObjectForm::SmartObjectForm() |
||
| 436 | */ |
||
| 437 | public function getForm( |
||
| 450 | |||
| 451 | /** |
||
| 452 | * @return array |
||
| 453 | */ |
||
| 454 | public function toArray() |
||
| 489 | |||
| 490 | /** |
||
| 491 | * add an error |
||
| 492 | * |
||
| 493 | * @param $err_str |
||
| 494 | * @param bool $prefix |
||
| 495 | * @internal param string $value error to add |
||
| 496 | * @access public |
||
| 497 | */ |
||
| 498 | public function setErrors($err_str, $prefix = false) |
||
| 511 | |||
| 512 | /** |
||
| 513 | * @param $field |
||
| 514 | * @param bool $required |
||
| 515 | */ |
||
| 516 | public function setFieldAsRequired($field, $required = true) |
||
| 526 | |||
| 527 | /** |
||
| 528 | * @param $field |
||
| 529 | */ |
||
| 530 | public function setFieldForSorting($field) |
||
| 540 | |||
| 541 | /** |
||
| 542 | * @return bool |
||
| 543 | */ |
||
| 544 | public function hasError() |
||
| 548 | |||
| 549 | /** |
||
| 550 | * @param $url |
||
| 551 | * @param $path |
||
| 552 | */ |
||
| 553 | public function setImageDir($url, $path) |
||
| 558 | |||
| 559 | /** |
||
| 560 | * Retreive the group that have been granted access to a specific permission for this object |
||
| 561 | * |
||
| 562 | * @param $group_perm |
||
| 563 | * @return string $group_perm name of the permission |
||
| 564 | */ |
||
| 565 | public function getGroupPerm($group_perm) |
||
| 582 | |||
| 583 | /** |
||
| 584 | * @param bool $path |
||
| 585 | * @return mixed |
||
| 586 | */ |
||
| 587 | public function getImageDir($path = false) |
||
| 595 | |||
| 596 | /** |
||
| 597 | * @param bool $path |
||
| 598 | * @return mixed |
||
| 599 | */ |
||
| 600 | public function getUploadDir($path = false) |
||
| 608 | |||
| 609 | /** |
||
| 610 | * @param string $key |
||
| 611 | * @param string $info |
||
| 612 | * @return array |
||
| 613 | */ |
||
| 614 | public function getVarInfo($key = '', $info = '') |
||
| 624 | |||
| 625 | /** |
||
| 626 | * Get the id of the object |
||
| 627 | * |
||
| 628 | * @return int id of this object |
||
| 629 | */ |
||
| 630 | public function id() |
||
| 634 | |||
| 635 | /** |
||
| 636 | * Return the value of the title field of this object |
||
| 637 | * |
||
| 638 | * @param string $format |
||
| 639 | * @return string |
||
| 640 | */ |
||
| 641 | public function title($format = 's') |
||
| 645 | |||
| 646 | /** |
||
| 647 | * Return the value of the title field of this object |
||
| 648 | * |
||
| 649 | * @return string |
||
| 650 | */ |
||
| 651 | public function summary() |
||
| 659 | |||
| 660 | /** |
||
| 661 | * Retreive the object admin side link, displayijng a SingleView page |
||
| 662 | * |
||
| 663 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 664 | * @return string user side link to the object |
||
| 665 | */ |
||
| 666 | public function getAdminViewItemLink($onlyUrl = false) |
||
| 672 | |||
| 673 | /** |
||
| 674 | * Retreive the object user side link |
||
| 675 | * |
||
| 676 | * @param bool $onlyUrl wether or not to return a simple URL or a full <a> link |
||
| 677 | * @return string user side link to the object |
||
| 678 | */ |
||
| 679 | public function getItemLink($onlyUrl = false) |
||
| 685 | |||
| 686 | /** |
||
| 687 | * @param bool $onlyUrl |
||
| 688 | * @param bool $withimage |
||
| 689 | * @param bool $userSide |
||
| 690 | * @return string |
||
| 691 | */ |
||
| 692 | public function getEditItemLink($onlyUrl = false, $withimage = true, $userSide = false) |
||
| 698 | |||
| 699 | /** |
||
| 700 | * @param bool $onlyUrl |
||
| 701 | * @param bool $withimage |
||
| 702 | * @param bool $userSide |
||
| 703 | * @return string |
||
| 704 | */ |
||
| 705 | public function getDeleteItemLink($onlyUrl = false, $withimage = false, $userSide = false) |
||
| 711 | |||
| 712 | /** |
||
| 713 | * @return string |
||
| 714 | */ |
||
| 715 | public function getPrintAndMailLink() |
||
| 721 | |||
| 722 | /** |
||
| 723 | * @param $sortsel |
||
| 724 | * @return array|bool |
||
| 725 | */ |
||
| 726 | public function getFieldsForSorting($sortsel) |
||
| 743 | |||
| 744 | /** |
||
| 745 | * @param $key |
||
| 746 | * @param $newType |
||
| 747 | */ |
||
| 748 | public function setType($key, $newType) |
||
| 752 | |||
| 753 | /** |
||
| 754 | * @param $key |
||
| 755 | * @param $info |
||
| 756 | * @param $value |
||
| 757 | */ |
||
| 758 | public function setVarInfo($key, $info, $value) |
||
| 762 | |||
| 763 | /** |
||
| 764 | * @param $key |
||
| 765 | * @param bool $editor |
||
| 766 | * @return string |
||
| 767 | */ |
||
| 768 | public function getValueFor($key, $editor = true) |
||
| 806 | |||
| 807 | /** |
||
| 808 | * clean values of all variables of the object for storage. |
||
| 809 | * also add slashes whereever needed |
||
| 810 | * |
||
| 811 | * We had to put this method in the SmartObject because the XOBJ_DTYPE_ARRAY does not work properly |
||
| 812 | * at least on PHP 5.1. So we have created a new type XOBJ_DTYPE_SIMPLE_ARRAY to handle 1 level array |
||
| 813 | * as a string separated by | |
||
| 814 | * |
||
| 815 | * @return bool true if successful |
||
| 816 | * @access public |
||
| 817 | */ |
||
| 818 | public function cleanVars() |
||
| 932 | |||
| 933 | /** |
||
| 934 | * returns a specific variable for the object in a proper format |
||
| 935 | * |
||
| 936 | * We had to put this method in the SmartObject because the XOBJ_DTYPE_ARRAY does not work properly |
||
| 937 | * at least on PHP 5.1. So we have created a new type XOBJ_DTYPE_SIMPLE_ARRAY to handle 1 level array |
||
| 938 | * as a string separated by | |
||
| 939 | * |
||
| 940 | * @access public |
||
| 941 | * @param string $key key of the object's variable to be returned |
||
| 942 | * @param string $format format to use for the output |
||
| 943 | * @return mixed formatted value of the variable |
||
| 944 | */ |
||
| 945 | public function getVar($key, $format = 's') |
||
| 1206 | |||
| 1207 | /** |
||
| 1208 | * @param $key |
||
| 1209 | */ |
||
| 1210 | public function doMakeFieldreadOnly($key) |
||
| 1217 | |||
| 1218 | /** |
||
| 1219 | * @param $key |
||
| 1220 | */ |
||
| 1221 | public function makeFieldReadOnly($key) |
||
| 1231 | |||
| 1232 | /** |
||
| 1233 | * @param $key |
||
| 1234 | */ |
||
| 1235 | public function doHideFieldFromForm($key) |
||
| 1241 | |||
| 1242 | /** |
||
| 1243 | * @param $key |
||
| 1244 | */ |
||
| 1245 | public function doHideFieldFromSingleView($key) |
||
| 1251 | |||
| 1252 | /** |
||
| 1253 | * @param $key |
||
| 1254 | */ |
||
| 1255 | public function hideFieldFromForm($key) |
||
| 1265 | |||
| 1266 | /** |
||
| 1267 | * @param $key |
||
| 1268 | */ |
||
| 1269 | public function hideFieldFromSingleView($key) |
||
| 1279 | |||
| 1280 | /** |
||
| 1281 | * @param $key |
||
| 1282 | */ |
||
| 1283 | public function doShowFieldOnForm($key) |
||
| 1289 | |||
| 1290 | /** |
||
| 1291 | * Display an automatic SingleView of the object, based on the displayOnSingleView param of each vars |
||
| 1292 | * |
||
| 1293 | * @param bool $fetchOnly if set to TRUE, then the content will be return, if set to FALSE, the content will be outputed |
||
| 1294 | * @param bool $userSide for futur use, to do something different on the user side |
||
| 1295 | * @param array $actions |
||
| 1296 | * @param bool $headerAsRow |
||
| 1297 | * @return string content of the template if $fetchOnly or nothing if !$fetchOnly |
||
| 1298 | */ |
||
| 1299 | public function displaySingleObject( |
||
| 1323 | |||
| 1324 | /** |
||
| 1325 | * @param $key |
||
| 1326 | */ |
||
| 1327 | public function doDisplayFieldOnSingleView($key) |
||
| 1333 | |||
| 1334 | /** |
||
| 1335 | * @param $field |
||
| 1336 | * @param bool $required |
||
| 1337 | */ |
||
| 1338 | public function doSetFieldAsRequired($field, $required = true) |
||
| 1342 | |||
| 1343 | /** |
||
| 1344 | * @param $field |
||
| 1345 | */ |
||
| 1346 | public function doSetFieldForSorting($field) |
||
| 1350 | |||
| 1351 | /** |
||
| 1352 | * @param $key |
||
| 1353 | */ |
||
| 1354 | public function showFieldOnForm($key) |
||
| 1364 | |||
| 1365 | /** |
||
| 1366 | * @param $key |
||
| 1367 | */ |
||
| 1368 | public function displayFieldOnSingleView($key) |
||
| 1378 | |||
| 1379 | /** |
||
| 1380 | * @param $key |
||
| 1381 | */ |
||
| 1382 | public function doSetAdvancedFormFields($key) |
||
| 1388 | |||
| 1389 | /** |
||
| 1390 | * @param $key |
||
| 1391 | */ |
||
| 1392 | public function setAdvancedFormFields($key) |
||
| 1402 | |||
| 1403 | /** |
||
| 1404 | * @param $key |
||
| 1405 | * @return mixed |
||
| 1406 | */ |
||
| 1407 | View Code Duplication | public function getUrlLinkObj($key) |
|
| 1417 | |||
| 1418 | /** |
||
| 1419 | * @param $urlLinkObj |
||
| 1420 | * @return mixed |
||
| 1421 | */ |
||
| 1422 | public function &storeUrlLinkObj($urlLinkObj) |
||
| 1428 | |||
| 1429 | /** |
||
| 1430 | * @param $key |
||
| 1431 | * @return mixed |
||
| 1432 | */ |
||
| 1433 | View Code Duplication | public function getFileObj($key) |
|
| 1443 | |||
| 1444 | /** |
||
| 1445 | * @param $fileObj |
||
| 1446 | * @return mixed |
||
| 1447 | */ |
||
| 1448 | public function &storeFileObj($fileObj) |
||
| 1454 | } |
||
| 1455 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.