geekwright /
XoopsCore
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 Tables 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 Tables, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 39 | class Tables |
||
| 40 | { |
||
| 41 | /** |
||
| 42 | * for add/alter column position |
||
| 43 | */ |
||
| 44 | const POSITION_FIRST = 1; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * @var Connection |
||
| 48 | */ |
||
| 49 | private $db; |
||
| 50 | |||
| 51 | /** |
||
| 52 | * @var Tables |
||
| 53 | */ |
||
| 54 | private $tables; |
||
| 55 | |||
| 56 | /** |
||
| 57 | * @var array Work queue |
||
| 58 | */ |
||
| 59 | private $queue; |
||
| 60 | |||
| 61 | /** |
||
| 62 | * @var string last error message |
||
| 63 | */ |
||
| 64 | protected $lastError; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * @var int last error number |
||
| 68 | */ |
||
| 69 | protected $lastErrNo; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * Constructor |
||
| 73 | * |
||
| 74 | */ |
||
| 75 | public function __construct() |
||
| 76 | { |
||
| 77 | Language::load('database', 'xmf'); |
||
| 78 | |||
| 79 | $this->db = Factory::getConnection(); |
||
| 80 | $this->queueReset(); |
||
| 81 | } |
||
| 82 | |||
| 83 | /** |
||
| 84 | * Return a table name, prefixed with site table prefix |
||
| 85 | * |
||
| 86 | * @param string $table table name to contain prefix |
||
| 87 | * |
||
| 88 | * @return string table name with prefix |
||
| 89 | */ |
||
| 90 | 1 | public function name($table) |
|
| 94 | |||
| 95 | /** |
||
| 96 | * Add new column for table to the work queue |
||
| 97 | * |
||
| 98 | * @param string $table table to contain the column |
||
| 99 | * @param string $column name of column to add |
||
| 100 | * @param array $attributes column_definition |
||
| 101 | * @param mixed $position FIRST, string of column name to add new |
||
| 102 | * column after, or null for natural append |
||
| 103 | * |
||
| 104 | * @return bool true if no errors, false if errors encountered |
||
| 105 | */ |
||
| 106 | public function addColumn($table, $column, $attributes, $position = null) |
||
| 107 | { |
||
| 108 | $columnDef=array( |
||
| 109 | 'name'=>$column, |
||
| 110 | 'position'=>$position, |
||
| 111 | 'attributes'=>$attributes |
||
| 112 | ); |
||
| 113 | |||
| 114 | // Find table def. |
||
| 115 | if (isset($this->tables[$table])) { |
||
| 116 | $tableDef = &$this->tables[$table]; |
||
| 117 | // Is this on a table we are adding? |
||
| 118 | if (isset($tableDef['create']) && $tableDef['create']) { |
||
| 119 | switch ($position) { |
||
| 120 | case Tables::POSITION_FIRST: |
||
| 121 | array_unshift($tableDef['columns'], $columnDef); |
||
| 122 | break; |
||
| 123 | case '': |
||
| 124 | case null: |
||
| 125 | case false: |
||
| 126 | array_push($tableDef['columns'], $columnDef); |
||
| 127 | break; |
||
| 128 | default: |
||
| 129 | // should be a column name to add after |
||
| 130 | // loop thru and find that column |
||
| 131 | $i=0; |
||
| 132 | foreach ($tableDef['columns'] as $col) { |
||
| 133 | ++$i; |
||
| 134 | if (strcasecmp($col['name'], $position)==0) { |
||
| 135 | array_splice($tableDef['columns'], $i, 0, array($columnDef)); |
||
| 136 | break; |
||
| 137 | } |
||
| 138 | } |
||
| 139 | } |
||
| 140 | |||
| 141 | return true; |
||
| 142 | } else { |
||
| 143 | View Code Duplication | foreach ($tableDef['columns'] as $col) { |
|
| 144 | if (strcasecmp($col['name'], $column)==0) { |
||
| 145 | return true; |
||
| 146 | } |
||
| 147 | } |
||
| 148 | View Code Duplication | switch ($position) { |
|
| 149 | case Tables::POSITION_FIRST: |
||
| 150 | $pos='FIRST'; |
||
| 151 | break; |
||
| 152 | case '': |
||
| 153 | case null: |
||
| 154 | case false: |
||
| 155 | $pos=''; |
||
| 156 | break; |
||
| 157 | default: |
||
| 158 | $pos="AFTER `{$position}`"; |
||
| 159 | } |
||
| 160 | $this->queue[]="ALTER TABLE `{$tableDef['name']}`" |
||
| 161 | . " ADD COLUMN {$column} {$columnDef['attributes']} {$pos} "; |
||
| 162 | } |
||
| 163 | } else { // no table established |
||
| 164 | $this->lastError = _DB_XMF_TABLE_IS_NOT_DEFINED; |
||
| 165 | $this->lastErrNo = -1; |
||
| 166 | |||
| 167 | return false; |
||
| 168 | } |
||
| 169 | |||
| 170 | return true; // exists or is added to queue |
||
| 171 | } |
||
| 172 | |||
| 173 | /** |
||
| 174 | * Add new primary key definition for table to work queue |
||
| 175 | * |
||
| 176 | * @param string $table table |
||
| 177 | * @param string $column column or comma separated list of columns |
||
| 178 | * to use as primary key |
||
| 179 | * |
||
| 180 | * @return bool true if no errors, false if errors encountered |
||
| 181 | */ |
||
| 182 | public function addPrimaryKey($table, $column) |
||
| 196 | |||
| 197 | /** |
||
| 198 | * Load table schema from database, or starts new empty schema if |
||
| 199 | * table does not exist |
||
| 200 | * |
||
| 201 | * @param string $table table |
||
| 202 | * |
||
| 203 | * @return bool true if no errors, false if errors encountered |
||
| 204 | */ |
||
| 205 | public function addTable($table) |
||
| 231 | |||
| 232 | /** |
||
| 233 | * AddTable only if it exists |
||
| 234 | * |
||
| 235 | * @param string $table table |
||
| 236 | * |
||
| 237 | * @return bool true if table exists, false otherwise |
||
| 238 | */ |
||
| 239 | public function useTable($table) |
||
| 251 | |||
| 252 | /** |
||
| 253 | * Get column attributes |
||
| 254 | * |
||
| 255 | * @param string $table table containing the column |
||
| 256 | * @param string $column column to alter |
||
| 257 | * |
||
| 258 | * @return string|bool attribute string, or false if error encountered |
||
| 259 | */ |
||
| 260 | public function getColumnAttributes($table, $column) |
||
| 275 | |||
| 276 | /** |
||
| 277 | * Get indexes for a table |
||
| 278 | * |
||
| 279 | * @param string $table table containing the column |
||
| 280 | * |
||
| 281 | * @return array|bool array of indexes, or false if error encountered |
||
| 282 | */ |
||
| 283 | public function getTableIndexes($table) |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Add alter column operation to the work queue |
||
| 295 | * |
||
| 296 | * @param string $table table containing the column |
||
| 297 | * @param string $column column to alter |
||
| 298 | * @param array $attributes new column_definition |
||
| 299 | * @param string $newName new name for column, blank to keep same |
||
| 300 | * @param mixed $position FIRST, string of column name to add new |
||
| 301 | * column after, or null for no change |
||
| 302 | * |
||
| 303 | * @return bool true if no errors, false if errors encountered |
||
| 304 | */ |
||
| 305 | public function alterColumn($table, $column, $attributes, $newName = '', $position = null) |
||
| 352 | |||
| 353 | /** |
||
| 354 | * Loads table schema from database, and adds newTable with that |
||
| 355 | * schema to the queue |
||
| 356 | * |
||
| 357 | * @param string $table existing table |
||
| 358 | * @param string $newTable new table |
||
| 359 | * @param bool $withData true to copy data, false for schema only |
||
| 360 | * |
||
| 361 | * @return bool true if no errors, false if errors encountered |
||
| 362 | */ |
||
| 363 | public function copyTable($table, $newTable, $withData = false) |
||
| 389 | |||
| 390 | /** |
||
| 391 | * Add new index definition for index to work queue |
||
| 392 | * |
||
| 393 | * @param string $name name of index to add |
||
| 394 | * @param string $table table indexed |
||
| 395 | * @param string $column column or comma separated list of columns |
||
| 396 | * to use as the key |
||
| 397 | * @param bool $unique true if index is to be unique |
||
| 398 | * |
||
| 399 | * @return bool true if no errors, false if errors encountered |
||
| 400 | */ |
||
| 401 | public function createIndex($name, $table, $column, $unique = false) |
||
| 417 | |||
| 418 | /** |
||
| 419 | * Add drop column operation to the work queue |
||
| 420 | * |
||
| 421 | * @param string $table table containing the column |
||
| 422 | * @param string $column column to drop |
||
| 423 | * |
||
| 424 | * @return bool true if no errors, false if errors encountered |
||
| 425 | */ |
||
| 426 | public function dropColumn($table, $column) |
||
| 442 | |||
| 443 | /** |
||
| 444 | * Add drop index operation to the work queue |
||
| 445 | * |
||
| 446 | * @param string $name name of index to drop |
||
| 447 | * @param string $table table indexed |
||
| 448 | * |
||
| 449 | * @return bool true if no errors, false if errors encountered |
||
| 450 | */ |
||
| 451 | public function dropIndex($name, $table) |
||
| 465 | |||
| 466 | /** |
||
| 467 | * Add drop for all (non-PRIMARY) keys for a table to the work |
||
| 468 | * queue. This can be used to clean up indexes with automatic names. |
||
| 469 | * |
||
| 470 | * @param string $table table indexed |
||
| 471 | * |
||
| 472 | * @return bool true if no errors, false if errors encountered |
||
| 473 | */ |
||
| 474 | public function dropIndexes($table) |
||
| 505 | |||
| 506 | /** |
||
| 507 | * Add drop of PRIMARY key for a table to the work queue |
||
| 508 | * |
||
| 509 | * @param string $table table |
||
| 510 | * |
||
| 511 | * @return bool true if no errors, false if errors encountered |
||
| 512 | */ |
||
| 513 | public function dropPrimaryKey($table) |
||
| 527 | |||
| 528 | /** |
||
| 529 | * Add drop of table to the work queue |
||
| 530 | * |
||
| 531 | * @param string $table table |
||
| 532 | * |
||
| 533 | * @return bool true if no errors, false if errors encountered |
||
| 534 | */ |
||
| 535 | public function dropTable($table) |
||
| 545 | |||
| 546 | |||
| 547 | /** |
||
| 548 | * Add rename table operation to the work queue |
||
| 549 | * |
||
| 550 | * @param string $table table |
||
| 551 | * @param string $newName new table name |
||
| 552 | * |
||
| 553 | * @return bool true if no errors, false if errors encountered |
||
| 554 | */ |
||
| 555 | View Code Duplication | public function renameTable($table, $newName) |
|
| 572 | |||
| 573 | /** |
||
| 574 | * Add alter table table_options (ENGINE, DEFAULT CHARSET, etc.) |
||
| 575 | * to work queue |
||
| 576 | * |
||
| 577 | * @param string $table table |
||
| 578 | * @param array $options table_options |
||
| 579 | * |
||
| 580 | * @return bool true if no errors, false if errors encountered |
||
| 581 | */ |
||
| 582 | View Code Duplication | public function setTableOptions($table, $options) |
|
| 598 | |||
| 599 | |||
| 600 | /** |
||
| 601 | * Clear the work queue |
||
| 602 | * |
||
| 603 | * @return void |
||
| 604 | */ |
||
| 605 | public function queueReset() |
||
| 610 | |||
| 611 | /** |
||
| 612 | * Execute the work queue |
||
| 613 | * |
||
| 614 | * @param bool $force true to force updates even if this is a 'GET' request |
||
| 615 | * |
||
| 616 | * @return bool true if no errors, false if errors encountered |
||
| 617 | */ |
||
| 618 | public function queueExecute($force = false) |
||
| 638 | |||
| 639 | |||
| 640 | /** |
||
| 641 | * Create DELETE statement and add to queue |
||
| 642 | * |
||
| 643 | * @param string $table table |
||
| 644 | * @param mixed $criteria string where clause or object criteria |
||
| 645 | * |
||
| 646 | * @return bool true if no errors, false if errors encountered |
||
| 647 | */ |
||
| 648 | public function delete($table, $criteria) |
||
| 668 | |||
| 669 | /** Create an INSERT SQL statement and add to queue. |
||
| 670 | * |
||
| 671 | * @param string $table table |
||
| 672 | * @param array $columns array of 'column'=>'value' entries |
||
| 673 | * |
||
| 674 | * @return boolean|null true if no errors, false if errors encountered |
||
| 675 | */ |
||
| 676 | public function insert($table, $columns) |
||
| 700 | |||
| 701 | /** |
||
| 702 | * Creates and executes an UPDATE SQL statement. |
||
| 703 | * |
||
| 704 | * @param string $table table |
||
| 705 | * @param array $columns array of 'column'=>'value' entries |
||
| 706 | * @param mixed $criteria string where clause or object criteria |
||
| 707 | * |
||
| 708 | * @return boolean|null true if no errors, false if errors encountered |
||
| 709 | */ |
||
| 710 | public function update($table, $columns, $criteria) |
||
| 739 | |||
| 740 | /** |
||
| 741 | * Add statement to Empty a table to the queue |
||
| 742 | * |
||
| 743 | * @param string $table table |
||
| 744 | * |
||
| 745 | * @return bool true if no errors, false if errors encountered |
||
| 746 | */ |
||
| 747 | public function truncate($table) |
||
| 761 | |||
| 762 | |||
| 763 | |||
| 764 | /** |
||
| 765 | * return SQL to create the table |
||
| 766 | * |
||
| 767 | * @param string $table table |
||
| 768 | * @param bool $prefixed true to return with table name prefixed |
||
| 769 | * |
||
| 770 | * @return string|null string SQL to create table, or null if errors encountered |
||
| 771 | */ |
||
| 772 | public function renderTableCreate($table, $prefixed = false) |
||
| 803 | |||
| 804 | /** |
||
| 805 | * execute an SQL statement |
||
| 806 | * |
||
| 807 | * @param string $sql SQL statement to execute |
||
| 808 | * @param bool $force true to use force updates even in safe requests |
||
| 809 | * |
||
| 810 | * @return mixed result Statement or false if error |
||
| 811 | */ |
||
| 812 | private function execSql($sql, $force = false) |
||
| 826 | |||
| 827 | /** |
||
| 828 | * fetch the next row of a result set |
||
| 829 | * |
||
| 830 | * @param Statement $result as returned by query |
||
| 831 | * |
||
| 832 | * @return mixed false on error |
||
| 833 | */ |
||
| 834 | private function fetch(Statement $result) |
||
| 838 | |||
| 839 | /** |
||
| 840 | * get table definition from INFORMATION_SCHEMA |
||
| 841 | * |
||
| 842 | * @param string $table table |
||
| 843 | * |
||
| 844 | * @return bool true if no errors and table is loaded, false if |
||
| 845 | * error presented. Error message in $this->lastError; |
||
| 846 | */ |
||
| 847 | private function getTable($table) |
||
| 933 | |||
| 934 | /** |
||
| 935 | * During processing, tables to be created are put in the queue as |
||
| 936 | * an array('createtable'=>tablename) since the definition is not |
||
| 937 | * complete. This method will expand those references to the full |
||
| 938 | * ddl to create the table. |
||
| 939 | * |
||
| 940 | * @return void |
||
| 941 | */ |
||
| 942 | private function expandQueue() |
||
| 952 | |||
| 953 | /** |
||
| 954 | * Return message from last error encountered |
||
| 955 | * |
||
| 956 | * @return string last error message |
||
| 957 | */ |
||
| 958 | public function getLastError() |
||
| 962 | |||
| 963 | /** |
||
| 964 | * Return code from last error encountered |
||
| 965 | * |
||
| 966 | * @return int last error number |
||
| 967 | */ |
||
| 968 | public function getLastErrNo() |
||
| 972 | |||
| 973 | /** |
||
| 974 | * dumpTables - development function to dump raw tables array |
||
| 975 | * |
||
| 976 | * @return array tables |
||
| 977 | */ |
||
| 978 | public function dumpTables() |
||
| 982 | |||
| 983 | /** |
||
| 984 | * dumpQueue - development function to dump the work queue |
||
| 985 | * |
||
| 986 | * @return array work queue |
||
| 987 | */ |
||
| 988 | public function dumpQueue() |
||
| 994 | } |
||
| 995 |
Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.
Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..