XoopsModules25x /
oledrion
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 Oledrion_XoopsPersistableObjectHandler 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 Oledrion_XoopsPersistableObjectHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 73 | class Oledrion_XoopsPersistableObjectHandler extends XoopsObjectHandler |
||
| 74 | { |
||
| 75 | /**#@+ |
||
| 76 | * Information about the class, the handler is managing |
||
| 77 | * |
||
| 78 | * @var string |
||
| 79 | */ |
||
| 80 | public $table; |
||
| 81 | protected $keyName; |
||
| 82 | protected $className; |
||
| 83 | protected $identifierName; |
||
| 84 | protected $cacheOptions = array(); |
||
| 85 | |||
| 86 | /**#@-*/ |
||
| 87 | |||
| 88 | /** |
||
| 89 | * Constructor - called from child classes |
||
| 90 | * @param object|XoopsDatabase $db {@link XoopsDatabase} |
||
| 91 | * object |
||
| 92 | * @param string $tablename Name of database table |
||
| 93 | * @param string $classname Name of Class, this handler is managing |
||
| 94 | * @param string $keyname Name of the property, holding the key |
||
| 95 | * @param string $idenfierName Name of the property, holding the label |
||
| 96 | * @param array $cacheOptions Optional, options for the cache |
||
| 97 | */ |
||
| 98 | public function __construct(XoopsDatabase $db, $tablename, $classname, $keyname, $idenfierName = '', $cacheOptions = null) |
||
| 122 | |||
| 123 | /** |
||
| 124 | * @param $cacheOptions |
||
| 125 | */ |
||
| 126 | public function setCachingOptions($cacheOptions) |
||
| 130 | |||
| 131 | /** |
||
| 132 | * Generates a unique ID for a Sql Query |
||
| 133 | * |
||
| 134 | * @param string $query The SQL query for which we want a unidque ID |
||
| 135 | * @param integer $start Which record to start at |
||
| 136 | * @param integer $limit Max number of objects to fetch |
||
| 137 | * @return string An MD5 of the query |
||
| 138 | */ |
||
| 139 | protected function _getIdForCache($query, $start, $limit) |
||
| 145 | |||
| 146 | /** |
||
| 147 | * create a new object |
||
| 148 | * |
||
| 149 | * @param bool $isNew Flag the new objects as "new"? |
||
| 150 | * |
||
| 151 | * @return object |
||
| 152 | */ |
||
| 153 | public function create($isNew = true) |
||
| 162 | |||
| 163 | /** |
||
| 164 | * retrieve an object |
||
| 165 | * |
||
| 166 | * @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 |
||
| 167 | * @param bool $as_object whether to return an object or an array |
||
| 168 | * @return mixed reference to the object, FALSE if failed |
||
| 169 | */ |
||
| 170 | public function get($id, $as_object = true) |
||
| 191 | |||
| 192 | /** |
||
| 193 | * retrieve objects from the database |
||
| 194 | * |
||
| 195 | * @param object $criteria {@link CriteriaElement} conditions to be met |
||
| 196 | * @param bool $id_as_key use the ID as key for the array? |
||
| 197 | * @param bool $as_object return an array of objects? |
||
| 198 | * |
||
| 199 | * @param string $fields |
||
| 200 | * @param bool $autoSort |
||
| 201 | * @return array |
||
| 202 | */ |
||
| 203 | public function getObjects( |
||
| 243 | |||
| 244 | /** |
||
| 245 | * Convert a database resultset to a returnable array |
||
| 246 | * |
||
| 247 | * @param object $result database resultset |
||
| 248 | * @param boolean $id_as_key - should NOT be used with joint keys |
||
| 249 | * @param boolean $as_object |
||
| 250 | * @param string $fields Requested fields from the query |
||
| 251 | * |
||
| 252 | * @return array |
||
| 253 | */ |
||
| 254 | protected function convertResultSet($result, $id_as_key = false, $as_object = true, $fields = '*') |
||
| 294 | |||
| 295 | /** |
||
| 296 | * get IDs of objects matching a condition |
||
| 297 | * |
||
| 298 | * @param object $criteria {@link CriteriaElement} to match |
||
| 299 | * @return array of object IDs |
||
| 300 | */ |
||
| 301 | public function getIds($criteria = null) |
||
| 337 | |||
| 338 | /** |
||
| 339 | * Retrieve a list of objects as arrays - DON'T USE WITH JOINT KEYS |
||
| 340 | * |
||
| 341 | * @param object $criteria {@link CriteriaElement} conditions to be met |
||
| 342 | * @return array |
||
| 343 | */ |
||
| 344 | public function getList($criteria = null) |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Retourne des éléments selon leur ID |
||
| 395 | * |
||
| 396 | * @param array $ids Les ID des éléments à retrouver |
||
| 397 | * @return array Tableau d'objets (clé = id key name) |
||
| 398 | */ |
||
| 399 | public function getItemsFromIds($ids) |
||
| 409 | |||
| 410 | /** |
||
| 411 | * count objects matching a condition |
||
| 412 | * |
||
| 413 | * @param object $criteria {@link CriteriaElement} to match |
||
| 414 | * @return int count of objects |
||
| 415 | */ |
||
| 416 | public function getCount($criteria = null) |
||
| 467 | |||
| 468 | /** |
||
| 469 | * Retourne le total d'un champ |
||
| 470 | * |
||
| 471 | * @param string $field Le champ dont on veut calculer le total |
||
| 472 | * @param object $criteria {@link CriteriaElement} to match |
||
| 473 | * @return integer le total |
||
| 474 | */ |
||
| 475 | public function getSum($field, $criteria = null) |
||
| 509 | |||
| 510 | /** |
||
| 511 | * delete an object from the database |
||
| 512 | * |
||
| 513 | * @param XoopsObject $obj reference to the object to delete |
||
| 514 | * @param bool $force |
||
| 515 | * @return bool FALSE if failed. |
||
| 516 | */ |
||
| 517 | public function delete(XoopsObject $obj, $force = false) |
||
| 544 | |||
| 545 | /** |
||
| 546 | * Quickly insert a record like this $myobjectHandler->quickInsert('field1' => field1value, 'field2' => $field2value) |
||
| 547 | * |
||
| 548 | * @param array $vars Array containing the fields name and value |
||
| 549 | * @param bool $force whether to force the query execution despite security settings |
||
| 550 | * @return bool @link insert's value |
||
| 551 | */ |
||
| 552 | public function quickInsert($vars = null, $force = true) |
||
| 564 | |||
| 565 | /** |
||
| 566 | * insert a new object in the database |
||
| 567 | * |
||
| 568 | * @param XoopsObject $obj reference to the object |
||
| 569 | * @param bool $force whether to force the query execution despite security settings |
||
| 570 | * @param bool $checkObject check if the object is dirty and clean the attributes |
||
| 571 | * @return bool FALSE if failed, TRUE if already present and unchanged or successful |
||
| 572 | */ |
||
| 573 | |||
| 574 | public function insert(XoopsObject $obj, $force = false, $checkObject = true) |
||
| 575 | { |
||
| 576 | if (false !== $checkObject) { |
||
| 577 | if (!is_object($obj)) { |
||
| 578 | trigger_error('Error, not object'); |
||
| 579 | |||
| 580 | return false; |
||
| 581 | } |
||
| 582 | /** |
||
| 583 | * @TODO: Change to if (!(class_exists($this->className) && $obj instanceof $this->className)) when going fully PHP5 |
||
| 584 | */ |
||
| 585 | if (!is_a($obj, $this->className)) { |
||
| 586 | $obj->setErrors(get_class($obj) . ' Differs from ' . $this->className); |
||
| 587 | |||
| 588 | return false; |
||
| 589 | } |
||
| 590 | if (!$obj->isDirty()) { |
||
| 591 | $obj->setErrors('Not dirty'); //will usually not be outputted as errors are not displayed when the method returns true, but it can be helpful when troubleshooting code - Mith |
||
| 592 | |||
| 593 | return true; |
||
| 594 | } |
||
| 595 | } |
||
| 596 | if (!$obj->cleanVars()) { |
||
| 597 | foreach ($obj->getErrors() as $oneerror) { |
||
| 598 | trigger_error($oneerror); |
||
| 599 | } |
||
| 600 | |||
| 601 | return false; |
||
| 602 | } |
||
| 603 | foreach ($obj->cleanVars as $k => $v) { |
||
| 604 | if ($obj->vars[$k]['data_type'] == XOBJ_DTYPE_INT) { |
||
| 605 | $cleanvars[$k] = (int)$v; |
||
| 606 | } elseif (is_array($v)) { |
||
| 607 | $cleanvars[$k] = $this->db->quoteString(implode(',', $v)); |
||
| 608 | } else { |
||
| 609 | $cleanvars[$k] = $this->db->quoteString($v); |
||
| 610 | } |
||
| 611 | } |
||
| 612 | if (isset($cleanvars['dohtml'])) { // Modification Hervé to be able to use dohtml |
||
| 613 | unset($cleanvars['dohtml']); |
||
| 614 | } |
||
| 615 | if ($obj->isNew()) { |
||
| 616 | if (!is_array($this->keyName)) { |
||
| 617 | if ($cleanvars[$this->keyName] < 1) { |
||
| 618 | $cleanvars[$this->keyName] = $this->db->genId($this->table . '_' . $this->keyName . '_seq'); |
||
| 619 | } |
||
| 620 | } |
||
| 621 | $sql = 'INSERT INTO ' . $this->table . ' (' . implode(',', array_keys($cleanvars)) . ') VALUES (' . implode(',', array_values($cleanvars)) . ')'; |
||
| 622 | } else { |
||
| 623 | $sql = 'UPDATE ' . $this->table . ' SET'; |
||
| 624 | foreach ($cleanvars as $key => $value) { |
||
| 625 | if ((!is_array($this->keyName) && $key == $this->keyName) |
||
| 626 | || (is_array($this->keyName) |
||
| 627 | && in_array($key, $this->keyName))) { |
||
| 628 | continue; |
||
| 629 | } |
||
| 630 | if (isset($notfirst)) { |
||
| 631 | $sql .= ','; |
||
| 632 | } |
||
| 633 | $sql .= ' ' . $key . ' = ' . $value; |
||
| 634 | $notfirst = true; |
||
| 635 | } |
||
| 636 | if (is_array($this->keyName)) { |
||
| 637 | $whereclause = ''; |
||
| 638 | $vnb = count($this->keyName); |
||
| 639 | for ($i = 0; $i < $vnb; ++$i) { |
||
| 640 | if ($i > 0) { |
||
| 641 | $whereclause .= ' AND '; |
||
| 642 | } |
||
| 643 | $whereclause .= $this->keyName[$i] . ' = ' . $obj->getVar($this->keyName[$i]); |
||
| 644 | } |
||
| 645 | } else { |
||
| 646 | $whereclause = $this->keyName . ' = ' . $obj->getVar($this->keyName); |
||
| 647 | } |
||
| 648 | $sql .= ' WHERE ' . $whereclause; |
||
| 649 | } |
||
| 650 | |||
| 651 | View Code Duplication | if (false !== $force) { |
|
| 652 | $result = $this->db->queryF($sql); |
||
| 653 | } else { |
||
| 654 | $result = $this->db->query($sql); |
||
| 655 | } |
||
| 656 | |||
| 657 | // Clear cache |
||
| 658 | $this->forceCacheClean(); |
||
| 659 | |||
| 660 | if (!$result) { |
||
| 661 | return false; |
||
| 662 | } |
||
| 663 | if ($obj->isNew() && !is_array($this->keyName)) { |
||
| 664 | $obj->assignVar($this->keyName, $this->db->getInsertId()); |
||
| 665 | } |
||
| 666 | |||
| 667 | return true; |
||
| 668 | } |
||
| 669 | |||
| 670 | /** |
||
| 671 | * Change a value for objects with a certain criteria |
||
| 672 | * |
||
| 673 | * @param string $fieldname Name of the field |
||
| 674 | * @param string $fieldvalue Value to write |
||
| 675 | * @param object $criteria {@link CriteriaElement} |
||
| 676 | * |
||
| 677 | * @param bool $force |
||
| 678 | * @return bool |
||
| 679 | */ |
||
| 680 | public function updateAll($fieldname, $fieldvalue, $criteria = null, $force = false) |
||
| 681 | { |
||
| 682 | $set_clause = $fieldname . ' = '; |
||
| 683 | if (is_numeric($fieldvalue)) { |
||
| 684 | $set_clause .= $fieldvalue; |
||
| 685 | } elseif (is_array($fieldvalue)) { |
||
| 686 | $set_clause .= $this->db->quoteString(implode(',', $fieldvalue)); |
||
| 687 | } else { |
||
| 688 | $set_clause .= $this->db->quoteString($fieldvalue); |
||
| 689 | } |
||
| 690 | $sql = 'UPDATE ' . $this->table . ' SET ' . $set_clause; |
||
| 691 | if (isset($criteria) && is_subclass_of($criteria, 'criteriaelement')) { |
||
| 692 | $sql .= ' ' . $criteria->renderWhere(); |
||
| 693 | } |
||
| 694 | if ($force) { |
||
| 695 | $result = $this->db->queryF($sql); |
||
| 696 | } else { |
||
| 697 | $result = $this->db->query($sql); |
||
| 698 | } |
||
| 699 | |||
| 700 | // Clear cache |
||
| 701 | $this->forceCacheClean(); |
||
| 702 | |||
| 703 | if (!$result) { |
||
| 704 | return false; |
||
| 705 | } |
||
| 706 | |||
| 707 | return true; |
||
| 708 | } |
||
| 709 | |||
| 710 | // check if target object is attempting to use duplicated info |
||
| 711 | |||
| 712 | /** |
||
| 713 | * @param $obj |
||
| 714 | * @param string $field |
||
| 715 | * @param string $error |
||
| 716 | * @return bool |
||
| 717 | */ |
||
| 718 | public function isDuplicated($obj, $field = '', $error = '') |
||
| 737 | |||
| 738 | /** |
||
| 739 | * delete all objects meeting the conditions |
||
| 740 | * |
||
| 741 | * @param object $criteria {@link CriteriaElement} with conditions to meet |
||
| 742 | * @return bool |
||
| 743 | */ |
||
| 744 | public function deleteAll($criteria = null) |
||
| 762 | |||
| 763 | /** |
||
| 764 | * Compare two objects and returns, in an array, the differences |
||
| 765 | * |
||
| 766 | * @param XoopsObject $old_object The first object to compare |
||
| 767 | * @param XoopsObject $new_object The new object |
||
| 768 | * @return array differences key = fieldname, value = array('old_value', 'new_value') |
||
| 769 | */ |
||
| 770 | public function compareObjects($old_object, $new_object) |
||
| 783 | |||
| 784 | /** |
||
| 785 | * Get distincted values of a field in the table |
||
| 786 | * |
||
| 787 | * @param string $field Field's name |
||
| 788 | * @param object $criteria {@link CriteriaElement} conditions to be met |
||
| 789 | * @param string $format Format in wich we want the datas |
||
| 790 | * @return array containing the distinct values |
||
| 791 | */ |
||
| 792 | public function getDistincts($field, $criteria = null, $format = 's') |
||
| 822 | |||
| 823 | /** |
||
| 824 | * A generic shortcut to getObjects |
||
| 825 | * |
||
| 826 | * @author Herve Thouzard - Instant Zero |
||
| 827 | * |
||
| 828 | * @param integer $start Starting position |
||
| 829 | * @param integer $limit Maximum count of elements to return |
||
| 830 | * @param string $sort Field to use for the sort |
||
| 831 | * @param string $order Sort order |
||
| 832 | * @param boolean $idAsKey Do we have to return an array whoses keys are the record's ID ? |
||
| 833 | * @return array Array of current objects |
||
| 834 | */ |
||
| 835 | public function getItems($start = 0, $limit = 0, $sort = '', $order = 'ASC', $idAsKey = true) |
||
| 854 | |||
| 855 | /** |
||
| 856 | * Forces the cache to be cleaned |
||
| 857 | */ |
||
| 858 | public function forceCacheClean() |
||
| 864 | } |
||
| 865 |
Loading history...
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.