mambax7 /
smartobject
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 PersistableObjectHandler 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 PersistableObjectHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace XoopsModules\Smartobject; |
||
| 30 | class PersistableObjectHandler extends \XoopsObjectHandler |
||
| 31 | { |
||
| 32 | public $_itemname; |
||
| 33 | |||
| 34 | /** |
||
| 35 | * Name of the table use to store this {@link SmartObject} |
||
| 36 | * |
||
| 37 | * Note that the name of the table needs to be free of the database prefix. |
||
| 38 | * For example "smartsection_categories" |
||
| 39 | * @var string |
||
| 40 | */ |
||
| 41 | public $table; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * Name of the table key that uniquely identify each {@link SmartObject} |
||
| 45 | * |
||
| 46 | * For example: "categoryid" |
||
| 47 | * @var string |
||
| 48 | */ |
||
| 49 | public $keyName; |
||
| 50 | |||
| 51 | /** |
||
| 52 | * Name of the class derived from {@link BaseSmartObject} and which this handler is handling |
||
| 53 | * |
||
| 54 | * Note that this string needs to be lowercase |
||
| 55 | * |
||
| 56 | * For example: "smartsectioncategory" |
||
| 57 | * @var string |
||
| 58 | */ |
||
| 59 | public $className; |
||
| 60 | |||
| 61 | /** |
||
| 62 | * Name of the field which properly identify the {@link SmartObject} |
||
| 63 | * |
||
| 64 | * For example: "name" (this will be the category's name) |
||
| 65 | * @var string |
||
| 66 | */ |
||
| 67 | public $identifierName; |
||
| 68 | |||
| 69 | /** |
||
| 70 | * Name of the field which will be use as a summary for the object |
||
| 71 | * |
||
| 72 | * For example: "summary" |
||
| 73 | * @var string |
||
| 74 | */ |
||
| 75 | public $summaryName; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * Page name use to basically manage and display the {@link SmartObject} |
||
| 79 | * |
||
| 80 | * This page needs to be the same in user side and admin side |
||
| 81 | * |
||
| 82 | * For example category.php - we will deduct smartsection/category.php as well as smartsection/admin/category.php |
||
| 83 | * @todo this could probably be automatically deducted from the class name - for example, the class SmartsectionCategory will have "category.php" as it's managing page |
||
| 84 | * @var string |
||
| 85 | */ |
||
| 86 | public $_page; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * Full path of the module using this {@link SmartObject} |
||
| 90 | * |
||
| 91 | * <code>XOOPS_URL . "/modules/smartsection/"</code> |
||
| 92 | * @todo this could probably be automatically deducted from the class name as it is always prefixed with the module name |
||
| 93 | * @var string |
||
| 94 | */ |
||
| 95 | public $_modulePath; |
||
| 96 | |||
| 97 | public $_moduleUrl; |
||
| 98 | |||
| 99 | public $_moduleName; |
||
| 100 | |||
| 101 | public $_uploadUrl; |
||
| 102 | |||
| 103 | public $_uploadPath; |
||
| 104 | |||
| 105 | public $_allowedMimeTypes = 0; |
||
| 106 | |||
| 107 | public $_maxFileSize = 1000000; |
||
| 108 | |||
| 109 | public $_maxWidth = 500; |
||
| 110 | |||
| 111 | public $_maxHeight = 500; |
||
| 112 | |||
| 113 | public $highlightFields = []; |
||
| 114 | |||
| 115 | /** |
||
| 116 | * Array containing the events name and functions |
||
| 117 | * |
||
| 118 | * @var array |
||
| 119 | */ |
||
| 120 | public $eventArray = []; |
||
| 121 | |||
| 122 | /** |
||
| 123 | * Array containing the permissions that this handler will manage on the objects |
||
| 124 | * |
||
| 125 | * @var array |
||
| 126 | */ |
||
| 127 | public $permissionsArray = false; |
||
| 128 | |||
| 129 | public $generalSQL = false; |
||
| 130 | |||
| 131 | public $_eventHooks = []; |
||
| 132 | public $_disabledEvents = []; |
||
| 133 | |||
| 134 | /** |
||
| 135 | * Constructor - called from child classes |
||
| 136 | * |
||
| 137 | * @param \XoopsDatabase $db {@link XoopsDatabase} |
||
| 138 | * @param string $itemname Name of the class derived from <a href='psi_element://SmartObject'>SmartObject</a> and which this handler is handling and which this handler is handling |
||
| 139 | * @param string $keyname Name of the table key that uniquely identify each {@link SmartObject} |
||
| 140 | * @param string $idenfierName Name of the field which properly identify the {@link SmartObject} |
||
| 141 | * @param string $summaryName |
||
| 142 | * @param string $modulename |
||
| 143 | * @internal param string $tablename Name of the table use to store this <a href='psi_element://SmartObject'>SmartObject</a> |
||
| 144 | * @internal param string $page Page name use to basically manage and display the <a href='psi_element://SmartObject'>SmartObject</a> |
||
| 145 | * @internal param string $moduleName name of the module |
||
| 146 | */ |
||
| 147 | public function __construct(\XoopsDatabase $db, $itemname, $keyname, $idenfierName, $summaryName, $modulename) |
||
| 166 | |||
| 167 | /** |
||
| 168 | * @param $event |
||
| 169 | * @param $method |
||
| 170 | */ |
||
| 171 | public function addEventHook($event, $method) |
||
| 175 | |||
| 176 | /** |
||
| 177 | * Add a permission that this handler will manage for its objects |
||
| 178 | * |
||
| 179 | * Example: $this->addPermission('view', _AM_SSHOP_CAT_PERM_READ, _AM_SSHOP_CAT_PERM_READ_DSC); |
||
| 180 | * |
||
| 181 | * @param string $perm_name name of the permission |
||
| 182 | * @param string $caption caption of the control that will be displayed in the form |
||
| 183 | * @param bool|string $description description of the control that will be displayed in the form |
||
| 184 | */ |
||
| 185 | public function addPermission($perm_name, $caption, $description = false) |
||
| 195 | |||
| 196 | /** |
||
| 197 | * @param $criteria |
||
| 198 | * @param $perm_name |
||
| 199 | * @return bool |
||
| 200 | */ |
||
| 201 | public function setGrantedObjectsCriteria($criteria, $perm_name) |
||
| 213 | |||
| 214 | /** |
||
| 215 | * @param bool $_uploadPath |
||
| 216 | * @param bool $_allowedMimeTypes |
||
| 217 | * @param bool $_maxFileSize |
||
| 218 | * @param bool $_maxWidth |
||
| 219 | * @param bool $_maxHeight |
||
| 220 | */ |
||
| 221 | public function setUploaderConfig( |
||
| 234 | |||
| 235 | /** |
||
| 236 | * create a new {@link Smartobject\BaseSmartObject} |
||
| 237 | * |
||
| 238 | * @param bool $isNew Flag the new objects as "new"? |
||
| 239 | * |
||
| 240 | * @return Smartobject\BaseSmartObject {@link Smartobject\BaseSmartObject} |
||
| 241 | */ |
||
| 242 | public function create($isNew = true) |
||
| 259 | |||
| 260 | /** |
||
| 261 | * @return string |
||
| 262 | */ |
||
| 263 | public function getImageUrl() |
||
| 267 | |||
| 268 | /** |
||
| 269 | * @return string |
||
| 270 | */ |
||
| 271 | public function getImagePath() |
||
| 280 | |||
| 281 | /** |
||
| 282 | * retrieve a {@link SmartObject} |
||
| 283 | * |
||
| 284 | * @param mixed $id ID of the object - or array of ids for joint keys. Joint keys MUST be given in the same order as in the constructor |
||
| 285 | * @param bool $as_object whether to return an object or an array |
||
| 286 | * @param bool $debug |
||
| 287 | * @param bool $criteria |
||
| 288 | * @return mixed reference to the <a href='psi_element://SmartObject'>SmartObject</a>, FALSE if failed |
||
| 289 | * FALSE if failed |
||
| 290 | */ |
||
| 291 | public function get($id, $as_object = true, $debug = false, $criteria = false) |
||
| 334 | |||
| 335 | /** |
||
| 336 | * retrieve a {@link SmartObject} |
||
| 337 | * |
||
| 338 | * @param mixed $id ID of the object - or array of ids for joint keys. Joint keys MUST be given in the same order as in the constructor |
||
| 339 | * @param bool $as_object whether to return an object or an array |
||
| 340 | * @return mixed reference to the {@link SmartObject}, FALSE if failed |
||
| 341 | */ |
||
| 342 | public function &getD($id, $as_object = true) |
||
| 346 | |||
| 347 | /** |
||
| 348 | * retrieve objects from the database |
||
| 349 | * |
||
| 350 | * @param null|\CriteriaElement $criteria {@link CriteriaElement} conditions to be met |
||
| 351 | * @param bool $id_as_key use the ID as key for the array? |
||
| 352 | * @param bool $as_object return an array of objects? |
||
| 353 | * |
||
| 354 | * @param bool $sql |
||
| 355 | * @param bool $debug |
||
| 356 | * @return array |
||
| 357 | */ |
||
| 358 | public function getObjects( |
||
| 393 | |||
| 394 | /** |
||
| 395 | * @param $sql |
||
| 396 | * @param $criteria |
||
| 397 | * @param bool $force |
||
| 398 | * @param bool $debug |
||
| 399 | * @return array |
||
| 400 | */ |
||
| 401 | public function query($sql, $criteria, $force = false, $debug = false) |
||
| 434 | |||
| 435 | /** |
||
| 436 | * retrieve objects with debug mode - so will show the query |
||
| 437 | * |
||
| 438 | * @param CriteriaElement $criteria {@link CriteriaElement} conditions to be met |
||
| 439 | * @param bool $id_as_key use the ID as key for the array? |
||
| 440 | * @param bool $as_object return an array of objects? |
||
| 441 | * |
||
| 442 | * @param bool $sql |
||
| 443 | * @return array |
||
| 444 | */ |
||
| 445 | public function getObjectsD(CriteriaElement $criteria = null, $id_as_key = false, $as_object = true, $sql = false) |
||
| 449 | |||
| 450 | /** |
||
| 451 | * @param $arrayObjects |
||
| 452 | * @return array|bool |
||
| 453 | */ |
||
| 454 | public function getObjectsAsArray($arrayObjects) |
||
| 466 | |||
| 467 | /** |
||
| 468 | * Convert a database resultset to a returnable array |
||
| 469 | * |
||
| 470 | * @param object $result database resultset |
||
| 471 | * @param bool $id_as_key - should NOT be used with joint keys |
||
| 472 | * @param bool $as_object |
||
| 473 | * |
||
| 474 | * @return array |
||
| 475 | */ |
||
| 476 | public function convertResultSet($result, $id_as_key = false, $as_object = true) |
||
| 505 | |||
| 506 | /** |
||
| 507 | * @param null $criteria |
||
| 508 | * @param int $limit |
||
| 509 | * @param int $start |
||
| 510 | * @return array |
||
| 511 | */ |
||
| 512 | public function getListD($criteria = null, $limit = 0, $start = 0) |
||
| 516 | |||
| 517 | /** |
||
| 518 | * Retrieve a list of objects as arrays - DON'T USE WITH JOINT KEYS |
||
| 519 | * |
||
| 520 | * @param CriteriaElement $criteria {@link CriteriaElement} conditions to be met |
||
| 521 | * @param int $limit Max number of objects to fetch |
||
| 522 | * @param int $start Which record to start at |
||
| 523 | * |
||
| 524 | * @param bool $debug |
||
| 525 | * @return array |
||
| 526 | */ |
||
| 527 | public function getList(CriteriaElement $criteria = null, $limit = 0, $start = 0, $debug = false) |
||
| 569 | |||
| 570 | /** |
||
| 571 | * count objects matching a condition |
||
| 572 | * |
||
| 573 | * @param CriteriaElement $criteria {@link CriteriaElement} to match |
||
| 574 | * @return int count of objects |
||
| 575 | */ |
||
| 576 | public function getCount(CriteriaElement $criteria = null) |
||
| 620 | |||
| 621 | /** |
||
| 622 | * delete an object from the database |
||
| 623 | * |
||
| 624 | * @param \XoopsObject $obj reference to the object to delete |
||
| 625 | * @param bool $force |
||
| 626 | * @return bool FALSE if failed. |
||
| 627 | */ |
||
| 628 | public function delete(\XoopsObject $obj, $force = false) |
||
| 665 | |||
| 666 | /** |
||
| 667 | * @param $event |
||
| 668 | */ |
||
| 669 | public function disableEvent($event) |
||
| 679 | |||
| 680 | /** |
||
| 681 | * @return array |
||
| 682 | */ |
||
| 683 | public function getPermissions() |
||
| 687 | |||
| 688 | /** |
||
| 689 | * insert a new object in the database |
||
| 690 | * |
||
| 691 | * @param \XoopsObject $obj reference to the object |
||
| 692 | * @param bool $force whether to force the query execution despite security settings |
||
| 693 | * @param bool $checkObject check if the object is dirty and clean the attributes |
||
| 694 | * @param bool $debug |
||
| 695 | * @return bool FALSE if failed, TRUE if already present and unchanged or successful |
||
| 696 | */ |
||
| 697 | public function insert(\XoopsObject $obj, $force = false, $checkObject = true, $debug = false) |
||
| 859 | |||
| 860 | /** |
||
| 861 | * @param $obj |
||
| 862 | * @param bool $force |
||
| 863 | * @param bool $checkObject |
||
| 864 | * @param bool $debug |
||
| 865 | * @return bool |
||
| 866 | */ |
||
| 867 | public function insertD($obj, $force = false, $checkObject = true, $debug = false) |
||
| 871 | |||
| 872 | /** |
||
| 873 | * Change a value for objects with a certain criteria |
||
| 874 | * |
||
| 875 | * @param string $fieldname Name of the field |
||
| 876 | * @param string $fieldvalue Value to write |
||
| 877 | * @param CriteriaElement $criteria {@link CriteriaElement} |
||
| 878 | * |
||
| 879 | * @param bool $force |
||
| 880 | * @return bool |
||
| 881 | */ |
||
| 882 | public function updateAll($fieldname, $fieldvalue, CriteriaElement $criteria = null, $force = false) |
||
| 907 | |||
| 908 | /** |
||
| 909 | * delete all objects meeting the conditions |
||
| 910 | * |
||
| 911 | * @param CriteriaElement $criteria {@link CriteriaElement} with conditions to meet |
||
| 912 | * @return bool |
||
| 913 | */ |
||
| 914 | |||
| 915 | public function deleteAll(CriteriaElement $criteria = null) |
||
| 930 | |||
| 931 | /** |
||
| 932 | * @return mixed |
||
| 933 | */ |
||
| 934 | public function getModuleInfo() |
||
| 938 | |||
| 939 | /** |
||
| 940 | * @return bool |
||
| 941 | */ |
||
| 942 | public function getModuleConfig() |
||
| 946 | |||
| 947 | /** |
||
| 948 | * @return string |
||
| 949 | */ |
||
| 950 | public function getModuleItemString() |
||
| 956 | |||
| 957 | /** |
||
| 958 | * @param $object |
||
| 959 | */ |
||
| 960 | public function updateCounter($object) |
||
| 968 | |||
| 969 | /** |
||
| 970 | * Execute the function associated with an event |
||
| 971 | * This method will check if the function is available |
||
| 972 | * |
||
| 973 | * @param string $event name of the event |
||
| 974 | * @param $executeEventObj |
||
| 975 | * @return mixed result of the execution of the function or FALSE if the function was not executed |
||
| 976 | * @internal param object $obj $object on which is performed the event |
||
| 977 | */ |
||
| 978 | public function executeEvent($event, &$executeEventObj) |
||
| 1000 | |||
| 1001 | /** |
||
| 1002 | * @param bool $withprefix |
||
| 1003 | * @return string |
||
| 1004 | */ |
||
| 1005 | public function getIdentifierName($withprefix = true) |
||
| 1013 | } |
||
| 1014 |
Loading history...
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountIdthat can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theidproperty of an instance of theAccountclass. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.