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 |
||
| 36 | class Tables |
||
| 37 | { |
||
| 38 | /** |
||
| 39 | * @var \XoopsDatabase |
||
| 40 | */ |
||
| 41 | protected $db; |
||
| 42 | |||
| 43 | /** |
||
| 44 | * @var string |
||
| 45 | */ |
||
| 46 | protected $databaseName; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @var array Tables |
||
| 50 | */ |
||
| 51 | protected $tables; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * @var array Work queue |
||
| 55 | */ |
||
| 56 | protected $queue; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var string last error message |
||
| 60 | */ |
||
| 61 | protected $lastError; |
||
| 62 | |||
| 63 | /** |
||
| 64 | * @var int last error number |
||
| 65 | */ |
||
| 66 | protected $lastErrNo; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * Constructor |
||
| 70 | * |
||
| 71 | */ |
||
| 72 | public function __construct() |
||
| 80 | |||
| 81 | /** |
||
| 82 | * Return a table name, prefixed with site table prefix |
||
| 83 | * |
||
| 84 | * @param string $table table name to contain prefix |
||
| 85 | * |
||
| 86 | * @return string table name with prefix |
||
| 87 | */ |
||
| 88 | protected function name($table) |
||
| 92 | |||
| 93 | /** |
||
| 94 | * Add new column for table to the work queue |
||
| 95 | * |
||
| 96 | * @param string $table table to contain the column |
||
| 97 | * @param string $column name of column to add |
||
| 98 | * @param string $attributes column_definition |
||
| 99 | * |
||
| 100 | * @return bool true if no errors, false if errors encountered |
||
| 101 | */ |
||
| 102 | public function addColumn($table, $column, $attributes) |
||
| 130 | |||
| 131 | /** |
||
| 132 | * Add new primary key definition for table to work queue |
||
| 133 | * |
||
| 134 | * @param string $table table |
||
| 135 | * @param string $column column or comma separated list of columns |
||
| 136 | * to use as primary key |
||
| 137 | * |
||
| 138 | * @return bool true if no errors, false if errors encountered |
||
| 139 | */ |
||
| 140 | public function addPrimaryKey($table, $column) |
||
| 161 | |||
| 162 | /** |
||
| 163 | * Add new index definition for index to work queue |
||
| 164 | * |
||
| 165 | * @param string $name name of index to add |
||
| 166 | * @param string $table table indexed |
||
| 167 | * @param string $column column or a comma separated list of columns |
||
| 168 | * to use as the key |
||
| 169 | * @param bool $unique true if index is to be unique |
||
| 170 | * |
||
| 171 | * @return bool true if no errors, false if errors encountered |
||
| 172 | */ |
||
| 173 | public function addIndex($name, $table, $column, $unique = false) |
||
| 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) |
||
| 234 | |||
| 235 | /** |
||
| 236 | * AddTable only if it exists |
||
| 237 | * |
||
| 238 | * @param string $table table |
||
| 239 | * |
||
| 240 | * @return bool true if table exists, false otherwise |
||
| 241 | */ |
||
| 242 | public function useTable($table) |
||
| 254 | |||
| 255 | /** |
||
| 256 | * Get column attributes |
||
| 257 | * |
||
| 258 | * @param string $table table containing the column |
||
| 259 | * @param string $column column to alter |
||
| 260 | * |
||
| 261 | * @return string|bool attribute string, or false if error encountered |
||
| 262 | */ |
||
| 263 | public function getColumnAttributes($table, $column) |
||
| 278 | |||
| 279 | /** |
||
| 280 | * Get indexes for a table |
||
| 281 | * |
||
| 282 | * @param string $table get indexes for this named table |
||
| 283 | * |
||
| 284 | * @return array|bool array of indexes, or false if error encountered |
||
| 285 | */ |
||
| 286 | public function getTableIndexes($table) |
||
| 295 | |||
| 296 | /** |
||
| 297 | * Add alter column operation to the work queue |
||
| 298 | * |
||
| 299 | * @param string $table table containing the column |
||
| 300 | * @param string $column column to alter |
||
| 301 | * @param string $attributes new column_definition |
||
| 302 | * @param string $newName new name for column, blank to keep same |
||
| 303 | * |
||
| 304 | * @return bool true if no errors, false if errors encountered |
||
| 305 | */ |
||
| 306 | public function alterColumn($table, $column, $attributes, $newName = '') |
||
| 336 | |||
| 337 | /** |
||
| 338 | * Loads table schema from database, and adds newTable with that |
||
| 339 | * schema to the queue |
||
| 340 | * |
||
| 341 | * @param string $table existing table |
||
| 342 | * @param string $newTable new table |
||
| 343 | * @param bool $withData true to copy data, false for schema only |
||
| 344 | * |
||
| 345 | * @return bool true if no errors, false if errors encountered |
||
| 346 | */ |
||
| 347 | public function copyTable($table, $newTable, $withData = false) |
||
| 372 | |||
| 373 | /** |
||
| 374 | * Add drop column operation to the work queue |
||
| 375 | * |
||
| 376 | * @param string $table table containing the column |
||
| 377 | * @param string $column column to drop |
||
| 378 | * |
||
| 379 | * @return bool true if no errors, false if errors encountered |
||
| 380 | */ |
||
| 381 | View Code Duplication | public function dropColumn($table, $column) |
|
| 393 | |||
| 394 | /** |
||
| 395 | * Add drop index operation to the work queue |
||
| 396 | * |
||
| 397 | * @param string $name name of index to drop |
||
| 398 | * @param string $table table indexed |
||
| 399 | * |
||
| 400 | * @return bool true if no errors, false if errors encountered |
||
| 401 | */ |
||
| 402 | View Code Duplication | public function dropIndex($name, $table) |
|
| 413 | |||
| 414 | /** |
||
| 415 | * Add drop for all (non-PRIMARY) keys for a table to the work |
||
| 416 | * queue. This can be used to clean up indexes with automatic names. |
||
| 417 | * |
||
| 418 | * @param string $table table indexed |
||
| 419 | * |
||
| 420 | * @return bool true if no errors, false if errors encountered |
||
| 421 | */ |
||
| 422 | public function dropIndexes($table) |
||
| 449 | |||
| 450 | /** |
||
| 451 | * Add drop of PRIMARY key for a table to the work queue |
||
| 452 | * |
||
| 453 | * @param string $table table |
||
| 454 | * |
||
| 455 | * @return bool true if no errors, false if errors encountered |
||
| 456 | */ |
||
| 457 | View Code Duplication | public function dropPrimaryKey($table) |
|
| 468 | |||
| 469 | /** |
||
| 470 | * Add drop of table to the work queue |
||
| 471 | * |
||
| 472 | * @param string $table table |
||
| 473 | * |
||
| 474 | * @return bool true if no errors, false if errors encountered |
||
| 475 | */ |
||
| 476 | View Code Duplication | public function dropTable($table) |
|
| 486 | |||
| 487 | |||
| 488 | /** |
||
| 489 | * Add rename table operation to the work queue |
||
| 490 | * |
||
| 491 | * @param string $table table |
||
| 492 | * @param string $newName new table name |
||
| 493 | * |
||
| 494 | * @return bool true if no errors, false if errors encountered |
||
| 495 | */ |
||
| 496 | public function renameTable($table, $newName) |
||
| 511 | |||
| 512 | /** |
||
| 513 | * Add alter table table_options (ENGINE, DEFAULT CHARSET, etc.) |
||
| 514 | * to work queue |
||
| 515 | * |
||
| 516 | * @param string $table table |
||
| 517 | * @param array $options table_options |
||
| 518 | * |
||
| 519 | * @return bool true if no errors, false if errors encountered |
||
| 520 | */ |
||
| 521 | public function setTableOptions($table, $options) |
||
| 538 | |||
| 539 | |||
| 540 | /** |
||
| 541 | * Clear the work queue |
||
| 542 | * |
||
| 543 | * @return void |
||
| 544 | */ |
||
| 545 | public function resetQueue() |
||
| 550 | |||
| 551 | /** |
||
| 552 | * Executes the work queue |
||
| 553 | * |
||
| 554 | * @param bool $force true to force updates even if this is a 'GET' request |
||
| 555 | * |
||
| 556 | * @return bool true if no errors, false if errors encountered |
||
| 557 | */ |
||
| 558 | public function executeQueue($force = false) |
||
| 578 | |||
| 579 | |||
| 580 | /** |
||
| 581 | * Create a DELETE statement and add it to the work queue |
||
| 582 | * |
||
| 583 | * @param string $table table |
||
| 584 | * @param string|CriteriaElement $criteria string where clause or object criteria |
||
| 585 | * |
||
| 586 | * @return bool true if no errors, false if errors encountered |
||
| 587 | */ |
||
| 588 | public function delete($table, $criteria) |
||
| 605 | |||
| 606 | /** Create an INSERT SQL statement and add it to the work queue. |
||
| 607 | * |
||
| 608 | * @param string $table table |
||
| 609 | * @param array $columns array of 'column'=>'value' entries |
||
| 610 | * |
||
| 611 | * @return boolean true if no errors, false if errors encountered |
||
| 612 | */ |
||
| 613 | public function insert($table, $columns) |
||
| 634 | |||
| 635 | /** |
||
| 636 | * Create an UPDATE SQL statement and add it to the work queue |
||
| 637 | * |
||
| 638 | * @param string $table table |
||
| 639 | * @param array $columns array of 'column'=>'value' entries |
||
| 640 | * @param string|CriteriaElement $criteria string where clause or object criteria |
||
| 641 | * |
||
| 642 | * @return boolean true if no errors, false if errors encountered |
||
| 643 | */ |
||
| 644 | public function update($table, $columns, $criteria) |
||
| 670 | |||
| 671 | /** |
||
| 672 | * Add statement to remove all rows from a table to the work queue |
||
| 673 | * |
||
| 674 | * @param string $table table |
||
| 675 | * |
||
| 676 | * @return bool true if no errors, false if errors encountered |
||
| 677 | */ |
||
| 678 | View Code Duplication | public function truncate($table) |
|
| 689 | |||
| 690 | |||
| 691 | |||
| 692 | /** |
||
| 693 | * return SQL to create the table |
||
| 694 | * |
||
| 695 | * This method does NOT modify the work queue |
||
| 696 | * |
||
| 697 | * @param string $table table |
||
| 698 | * @param bool $prefixed true to return with table name prefixed |
||
| 699 | * |
||
| 700 | * @return string|false string SQL to create table, or false if errors encountered |
||
|
|
|||
| 701 | */ |
||
| 702 | protected function renderTableCreate($table, $prefixed = false) |
||
| 730 | |||
| 731 | /** |
||
| 732 | * execute an SQL statement |
||
| 733 | * |
||
| 734 | * @param string $sql SQL statement to execute |
||
| 735 | * @param bool $force true to use force updates even in safe requests |
||
| 736 | * |
||
| 737 | * @return mixed result resource if no error, |
||
| 738 | * true if no error but no result |
||
| 739 | * false if error encountered. |
||
| 740 | * Any error message is in $this->lastError; |
||
| 741 | */ |
||
| 742 | protected function execSql($sql, $force = false) |
||
| 757 | |||
| 758 | /** |
||
| 759 | * fetch the next row of a result set |
||
| 760 | * |
||
| 761 | * @param resource $result as returned by query |
||
| 762 | * |
||
| 763 | * @return mixed false on error |
||
| 764 | */ |
||
| 765 | protected function fetch($result) |
||
| 769 | |||
| 770 | /** |
||
| 771 | * get table definition from INFORMATION_SCHEMA |
||
| 772 | * |
||
| 773 | * @param string $table table |
||
| 774 | * |
||
| 775 | * @return array|bool table definition array if table exists, true if table not defined, or |
||
| 776 | * false on error. Error message in $this->lastError; |
||
| 777 | */ |
||
| 778 | protected function getTable($table) |
||
| 861 | |||
| 862 | /** |
||
| 863 | * During processing, tables to be created are put in the queue as |
||
| 864 | * an array('createtable' => tablename) since the definition is not |
||
| 865 | * complete. This method will expand those references to the full |
||
| 866 | * ddl to create the table. |
||
| 867 | * |
||
| 868 | * @return void |
||
| 869 | */ |
||
| 870 | protected function expandQueue() |
||
| 880 | |||
| 881 | /** |
||
| 882 | * Return message from last error encountered |
||
| 883 | * |
||
| 884 | * @return string last error message |
||
| 885 | */ |
||
| 886 | public function getLastError() |
||
| 890 | |||
| 891 | /** |
||
| 892 | * Return code from last error encountered |
||
| 893 | * |
||
| 894 | * @return int last error number |
||
| 895 | */ |
||
| 896 | public function getLastErrNo() |
||
| 900 | |||
| 901 | /** |
||
| 902 | * dumpTables - development function to dump raw tables array |
||
| 903 | * |
||
| 904 | * @return array tables |
||
| 905 | */ |
||
| 906 | public function dumpTables() |
||
| 910 | |||
| 911 | /** |
||
| 912 | * dumpQueue - development function to dump the work queue |
||
| 913 | * |
||
| 914 | * @return array work queue |
||
| 915 | */ |
||
| 916 | public function dumpQueue() |
||
| 922 | |||
| 923 | /** |
||
| 924 | * Set lastError as table not established |
||
| 925 | * |
||
| 926 | * @return false |
||
| 927 | */ |
||
| 928 | protected function tableNotEstablished() |
||
| 934 | } |
||
| 935 |
This check compares the return type specified in the
@returnannotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.