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 XoopsGroupPermHandler 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 XoopsGroupPermHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 118 | class XoopsGroupPermHandler extends XoopsObjectHandler |
||
| 119 | { |
||
| 120 | /** |
||
| 121 | * This should be here, since this really should be a XoopsPersistableObjectHandler |
||
| 122 | * Here, we fake it for future compatibility |
||
| 123 | * |
||
| 124 | * @var string table name |
||
| 125 | */ |
||
| 126 | public $table; |
||
| 127 | |||
| 128 | public function __construct(XoopsDatabase $db) |
||
| 133 | |||
| 134 | /** |
||
| 135 | * Create a new {@link XoopsGroupPerm} |
||
| 136 | * |
||
| 137 | * @param bool $isNew |
||
| 138 | * |
||
| 139 | * @return bool $isNew Flag the object as "new"? |
||
| 140 | */ |
||
| 141 | public function create($isNew = true) |
||
| 150 | |||
| 151 | /** |
||
| 152 | * Retrieve a group permission |
||
| 153 | * |
||
| 154 | * @param int $id ID |
||
| 155 | * |
||
| 156 | * @return XoopsGroupPerm {@link XoopsGroupPerm}, FALSE on fail |
||
| 157 | */ |
||
| 158 | public function get($id) |
||
| 176 | |||
| 177 | /** |
||
| 178 | * Store a {@link XoopsGroupPerm} |
||
| 179 | * |
||
| 180 | * @param XoopsObject|XoopsGroupPerm $perm a XoopsGroupPerm object |
||
| 181 | * |
||
| 182 | * @return bool true on success, otherwise false |
||
| 183 | */ |
||
| 184 | View Code Duplication | public function insert(XoopsObject $perm) |
|
| 215 | |||
| 216 | /** |
||
| 217 | * Delete a {@link XoopsGroupPerm} |
||
| 218 | * |
||
| 219 | * @param XoopsObject|XoopsGroupPerm $perm a XoopsGroupPerm object |
||
| 220 | * |
||
| 221 | * @return bool true on success, otherwise false |
||
| 222 | */ |
||
| 223 | public function delete(XoopsObject $perm) |
||
| 236 | |||
| 237 | /** |
||
| 238 | * Retrieve multiple {@link XoopsGroupPerm}s |
||
| 239 | * |
||
| 240 | * @param CriteriaElement $criteria {@link CriteriaElement} |
||
| 241 | * @param bool $id_as_key Use IDs as array keys? |
||
| 242 | * |
||
| 243 | * @return array Array of {@link XoopsGroupPerm}s |
||
| 244 | */ |
||
| 245 | View Code Duplication | public function getObjects(CriteriaElement $criteria = null, $id_as_key = false) |
|
| 272 | |||
| 273 | /** |
||
| 274 | * Count some {@link XoopsGroupPerm}s |
||
| 275 | * |
||
| 276 | * @param CriteriaElement $criteria {@link CriteriaElement} |
||
| 277 | * |
||
| 278 | * @return int |
||
| 279 | */ |
||
| 280 | View Code Duplication | public function getCount(CriteriaElement $criteria = null) |
|
| 294 | |||
| 295 | /** |
||
| 296 | * Delete all permissions by a certain criteria |
||
| 297 | * |
||
| 298 | * @param CriteriaElement $criteria {@link CriteriaElement} |
||
| 299 | * |
||
| 300 | * @return bool TRUE on success |
||
| 301 | */ |
||
| 302 | View Code Duplication | public function deleteAll(CriteriaElement $criteria = null) |
|
| 314 | |||
| 315 | /** |
||
| 316 | * Delete all module specific permissions assigned for a group |
||
| 317 | * |
||
| 318 | * @param int $gperm_groupid ID of a group |
||
| 319 | * @param int $gperm_modid ID of a module |
||
| 320 | * |
||
| 321 | * @return bool TRUE on success |
||
| 322 | */ |
||
| 323 | public function deleteByGroup($gperm_groupid, $gperm_modid = null) |
||
| 332 | |||
| 333 | /** |
||
| 334 | * Delete all module specific permissions |
||
| 335 | * |
||
| 336 | * @param int $gperm_modid ID of a module |
||
| 337 | * @param string $gperm_name Name of a module permission |
||
| 338 | * @param int $gperm_itemid ID of a module item |
||
| 339 | * |
||
| 340 | * @return bool TRUE on success |
||
| 341 | */ |
||
| 342 | View Code Duplication | public function deleteByModule($gperm_modid, $gperm_name = null, $gperm_itemid = null) |
|
| 354 | |||
| 355 | /** |
||
| 356 | * Check permission |
||
| 357 | * |
||
| 358 | * @param string $gperm_name Name of permission |
||
| 359 | * @param int $gperm_itemid ID of an item |
||
| 360 | * @param int /array $gperm_groupid A group ID or an array of group IDs |
||
| 361 | * @param int $gperm_modid ID of a module |
||
| 362 | * @param bool $trueifadmin Returns true for admin groups |
||
| 363 | * |
||
| 364 | * @return bool TRUE if permission is enabled |
||
| 365 | */ |
||
| 366 | public function checkRight($gperm_name, $gperm_itemid, $gperm_groupid, $gperm_modid = 1, $trueifadmin = true) |
||
| 393 | |||
| 394 | /** |
||
| 395 | * Add a permission |
||
| 396 | * |
||
| 397 | * @param string $gperm_name Name of permission |
||
| 398 | * @param int $gperm_itemid ID of an item |
||
| 399 | * @param int $gperm_groupid ID of a group |
||
| 400 | * @param int $gperm_modid ID of a module |
||
| 401 | * |
||
| 402 | * @return bool TRUE if success |
||
| 403 | */ |
||
| 404 | public function addRight($gperm_name, $gperm_itemid, $gperm_groupid, $gperm_modid = 1) |
||
| 414 | |||
| 415 | /** |
||
| 416 | * Get all item IDs that a group is assigned a specific permission |
||
| 417 | * |
||
| 418 | * @param string $gperm_name Name of permission |
||
| 419 | * @param int /array $gperm_groupid A group ID or an array of group IDs |
||
| 420 | * @param int $gperm_modid ID of a module |
||
| 421 | * |
||
| 422 | * @return array array of item IDs |
||
| 423 | */ |
||
| 424 | public function getItemIds($gperm_name, $gperm_groupid, $gperm_modid = 1) |
||
| 445 | |||
| 446 | /** |
||
| 447 | * Get all group IDs assigned a specific permission for a particular item |
||
| 448 | * |
||
| 449 | * @param string $gperm_name Name of permission |
||
| 450 | * @param int $gperm_itemid ID of an item |
||
| 451 | * @param int $gperm_modid ID of a module |
||
| 452 | * |
||
| 453 | * @return array array of group IDs |
||
| 454 | */ |
||
| 455 | public function getGroupIds($gperm_name, $gperm_itemid, $gperm_modid = 1) |
||
| 468 | } |
||
| 469 |
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.