allejo /
bzion
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 QueryBuilder 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 QueryBuilder, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 26 | class QueryBuilder implements Countable |
||
| 27 | { |
||
| 28 | /** |
||
| 29 | * The type of the model we're building a query for |
||
| 30 | * @var string |
||
| 31 | */ |
||
| 32 | protected $type; |
||
| 33 | |||
| 34 | protected $tableAlias = ''; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * The columns that the model provided us |
||
| 38 | * @var array |
||
| 39 | */ |
||
| 40 | protected $columns = array('id' => 'id'); |
||
| 41 | |||
| 42 | protected $extraColumns = ''; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * The conditions to include in WHERE |
||
| 46 | * @var string[] |
||
| 47 | */ |
||
| 48 | protected $conditions = array(); |
||
| 49 | |||
| 50 | /** |
||
| 51 | * The MySQL value parameters |
||
| 52 | * @var array |
||
| 53 | */ |
||
| 54 | protected $parameters = array(); |
||
| 55 | |||
| 56 | /** |
||
| 57 | * The MySQL value parameters for pagination |
||
| 58 | * @var array |
||
| 59 | */ |
||
| 60 | protected $paginationParameters = array(); |
||
| 61 | |||
| 62 | /** |
||
| 63 | * Extra MySQL query string to pass |
||
| 64 | * @var string |
||
| 65 | */ |
||
| 66 | protected $extras = ''; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * Extra MySQL query groupby string to pass |
||
| 70 | * @var string |
||
| 71 | */ |
||
| 72 | protected $groupQuery = ''; |
||
| 73 | |||
| 74 | /** |
||
| 75 | * A column based on which we should sort the results |
||
| 76 | * @var string|null |
||
| 77 | */ |
||
| 78 | private $sortBy = null; |
||
| 79 | |||
| 80 | /** |
||
| 81 | * Whether to reverse the results |
||
| 82 | * @var bool |
||
| 83 | */ |
||
| 84 | private $reverseSort = false; |
||
| 85 | |||
| 86 | /** |
||
| 87 | * The currently selected column |
||
| 88 | * @var string|null |
||
| 89 | */ |
||
| 90 | private $currentColumn = null; |
||
| 91 | |||
| 92 | /** |
||
| 93 | * The currently selected column without the table name (unless it was |
||
| 94 | * explicitly provided) |
||
| 95 | * @var string|null |
||
| 96 | */ |
||
| 97 | protected $currentColumnRaw = null; |
||
| 98 | |||
| 99 | /** |
||
| 100 | * A column to consider the name of the model |
||
| 101 | * @var string|null |
||
| 102 | */ |
||
| 103 | private $nameColumn = null; |
||
| 104 | |||
| 105 | /** |
||
| 106 | * The page to return |
||
| 107 | * @var int|null |
||
| 108 | */ |
||
| 109 | private $page = null; |
||
| 110 | |||
| 111 | /** |
||
| 112 | * Whether the ID of the first/last element has been provided |
||
| 113 | * @var bool |
||
| 114 | */ |
||
| 115 | private $limited = false; |
||
| 116 | |||
| 117 | /** |
||
| 118 | * The number of elements on every page |
||
| 119 | * @var int |
||
| 120 | */ |
||
| 121 | protected $resultsPerPage = 30; |
||
| 122 | |||
| 123 | /** |
||
| 124 | * Create a new QueryBuilder |
||
| 125 | * |
||
| 126 | * A new query builder should be created on a static getQueryBuilder() |
||
| 127 | * method on each model. The options array can contain the following |
||
| 128 | * properties: |
||
| 129 | * |
||
| 130 | * - `columns`: An associative array - the key of each entry is the column |
||
| 131 | * name that will be used by other methods, while the value is |
||
| 132 | * is the column name that is used in the database structure |
||
| 133 | * |
||
| 134 | * - `activeStatuses`: If the model has a status column, this should be |
||
| 135 | * a list of values that make the entry be considered |
||
| 136 | * "active" |
||
| 137 | * |
||
| 138 | * - `name`: The name of the column which represents the name of the object |
||
| 139 | * |
||
| 140 | * @param string $type The type of the Model (e.g. "Player" or "Match") |
||
| 141 | * @param array $options The options to pass to the builder (see above) |
||
| 142 | */ |
||
| 143 | 4 | public function __construct($type, $options = array()) |
|
| 155 | |||
| 156 | /** |
||
| 157 | * Select a column |
||
| 158 | * |
||
| 159 | * `$queryBuilder->where('username')->equals('administrator');` |
||
| 160 | * |
||
| 161 | * @param string $column The column to select |
||
| 162 | * @return static |
||
| 163 | */ |
||
| 164 | 4 | public function where($column) |
|
| 174 | |||
| 175 | /** |
||
| 176 | * Request that a column equals a string (case-insensitive) |
||
| 177 | * |
||
| 178 | * @param string $string The string that the column's value should equal to |
||
| 179 | * @return static |
||
| 180 | */ |
||
| 181 | 2 | public function equals($string) |
|
| 187 | |||
| 188 | /** |
||
| 189 | * Request that a column doesNOT equals a string (case-insensitive) |
||
| 190 | * |
||
| 191 | * @param string $string The string that the column's value should equal to |
||
| 192 | * @return static |
||
| 193 | */ |
||
| 194 | 1 | public function notEquals($string) |
|
| 200 | |||
| 201 | /** |
||
| 202 | * Request that a column is greater than a quantity |
||
| 203 | * |
||
| 204 | * @param string $quantity The quantity to test against |
||
| 205 | * @return static |
||
| 206 | */ |
||
| 207 | public function greaterThan($quantity) |
||
| 213 | |||
| 214 | /** |
||
| 215 | * Request that a column is less than a quantity |
||
| 216 | * |
||
| 217 | * @param string $quantity The quantity to test against |
||
| 218 | * @return static |
||
| 219 | */ |
||
| 220 | public function lessThan($quantity) |
||
| 226 | |||
| 227 | /** |
||
| 228 | * Request that a timestamp is before the specified time |
||
| 229 | * |
||
| 230 | * @param string|TimeDate $time The timestamp to compare to |
||
| 231 | * @param bool $inclusive Whether to include the given timestamp |
||
| 232 | * @param bool $reverse Whether to reverse the results |
||
| 233 | * |
||
| 234 | * @return static |
||
| 235 | */ |
||
| 236 | public function isBefore($time, $inclusive = false, $reverse = false) |
||
| 240 | |||
| 241 | /** |
||
| 242 | * Request that a timestamp is after the specified time |
||
| 243 | * |
||
| 244 | * @param string|TimeDate $time The timestamp to compare to |
||
| 245 | * @param bool $inclusive Whether to include the given timestamp |
||
| 246 | * @param bool $reverse Whether to reverse the results |
||
| 247 | * |
||
| 248 | * @return static |
||
| 249 | */ |
||
| 250 | 1 | public function isAfter($time, $inclusive = false, $reverse = false) |
|
| 263 | |||
| 264 | /** |
||
| 265 | * Request that a column equals a number |
||
| 266 | * |
||
| 267 | * @param int|Model|null $number The number that the column's value should |
||
| 268 | * equal to. If a Model is provided, use the |
||
| 269 | * model's ID, while null values are ignored. |
||
| 270 | * @return static |
||
| 271 | */ |
||
| 272 | 1 | View Code Duplication | public function is($number) |
| 286 | |||
| 287 | /** |
||
| 288 | * Request that a column equals one of some strings |
||
| 289 | * |
||
| 290 | * @todo Improve for PDO |
||
| 291 | * @param string[] $strings The list of accepted values for the column |
||
| 292 | * @return static |
||
| 293 | */ |
||
| 294 | 3 | public function isOneOf($strings) |
|
| 306 | |||
| 307 | /** |
||
| 308 | * Request that a column value starts with a string (case-insensitive) |
||
| 309 | * |
||
| 310 | * @param string $string The substring that the column's value should start with |
||
| 311 | * @return static |
||
| 312 | */ |
||
| 313 | public function startsWith($string) |
||
| 319 | |||
| 320 | /** |
||
| 321 | * Request that a specific model is not returned |
||
| 322 | * |
||
| 323 | * @param Model|int $model The ID or model you don't want to get |
||
| 324 | * @return static |
||
| 325 | */ |
||
| 326 | View Code Duplication | public function except($model) |
|
| 337 | |||
| 338 | /** |
||
| 339 | * Return the results sorted by the value of a column |
||
| 340 | * |
||
| 341 | * @param string $column The column based on which the results should be ordered |
||
| 342 | * @return static |
||
| 343 | */ |
||
| 344 | 2 | public function sortBy($column) |
|
| 354 | |||
| 355 | /** |
||
| 356 | * Reverse the order |
||
| 357 | * |
||
| 358 | * Note: This only works if you have specified a column in the sortBy() method |
||
| 359 | * |
||
| 360 | * @return static |
||
| 361 | */ |
||
| 362 | 2 | public function reverse() |
|
| 368 | |||
| 369 | /** |
||
| 370 | * Specify the number of results per page |
||
| 371 | * |
||
| 372 | * @param int $count The number of results |
||
| 373 | * @return static |
||
| 374 | */ |
||
| 375 | 2 | public function limit($count) |
|
| 381 | |||
| 382 | /** |
||
| 383 | * Only show results from a specific page |
||
| 384 | * |
||
| 385 | * @param int|null $page The page number (or null to show all pages - counting starts from 0) |
||
| 386 | * @return static |
||
| 387 | */ |
||
| 388 | 2 | public function fromPage($page) |
|
| 394 | |||
| 395 | /** |
||
| 396 | * End with a specific result |
||
| 397 | * |
||
| 398 | * @param int|Model $model The model (or database ID) after the first result |
||
| 399 | * @param bool $inclusive Whether to include the provided model |
||
| 400 | * @param bool $reverse Whether to reverse the results |
||
| 401 | * @return static |
||
| 402 | */ |
||
| 403 | 1 | public function endAt($model, $inclusive = false, $reverse = false) |
|
| 407 | |||
| 408 | /** |
||
| 409 | * Start with a specific result |
||
| 410 | * |
||
| 411 | * @param int|Model $model The model (or database ID) before the first result |
||
| 412 | * @param bool $inclusive Whether to include the provided model |
||
| 413 | * @param bool $reverse Whether to reverse the results |
||
| 414 | * @return static |
||
| 415 | */ |
||
| 416 | 1 | public function startAt($model, $inclusive = false, $reverse = false) |
|
| 443 | |||
| 444 | /** |
||
| 445 | * Request that only "active" Models should be returned |
||
| 446 | * |
||
| 447 | * @return static |
||
| 448 | */ |
||
| 449 | 3 | public function active() |
|
| 459 | |||
| 460 | /** |
||
| 461 | * Make sure that Models invisible to a player are not returned |
||
| 462 | * |
||
| 463 | * Note that this method does not take PermissionModel::canBeSeenBy() into |
||
| 464 | * consideration for performance purposes, so you will have to override this |
||
| 465 | * in your query builder if necessary. |
||
| 466 | * |
||
| 467 | * @param Player $player The player in question |
||
| 468 | * @param bool $showDeleted false to hide deleted models even from admins |
||
| 469 | * @return static |
||
| 470 | */ |
||
| 471 | 1 | public function visibleTo($player, $showDeleted = false) |
|
| 489 | |||
| 490 | /** |
||
| 491 | * Perform the query and get back the results in an array of names |
||
| 492 | * |
||
| 493 | * @return string[] An array of the type $id => $name |
||
| 494 | */ |
||
| 495 | 1 | public function getNames() |
|
| 505 | |||
| 506 | /** |
||
| 507 | * Perform the query and get back the results in a list of arrays |
||
| 508 | * |
||
| 509 | * @todo Play with partial models? |
||
| 510 | * @param string|string[] $columns The column(s) that should be returned |
||
| 511 | * @return array[] |
||
| 512 | */ |
||
| 513 | 1 | public function getArray($columns) |
|
| 523 | |||
| 524 | /** |
||
| 525 | * An alias for QueryBuilder::getModels(), with fast fetching on by default |
||
| 526 | * and no return of results |
||
| 527 | * |
||
| 528 | * @param bool $fastFetch Whether to perform one query to load all |
||
| 529 | * the model data instead of fetching them |
||
| 530 | * one by one |
||
| 531 | * @return void |
||
| 532 | */ |
||
| 533 | public function addToCache($fastFetch = true) |
||
| 537 | |||
| 538 | /** |
||
| 539 | * Perform the query and get the results as Models |
||
| 540 | * |
||
| 541 | * @todo Fix fast fetch for queries with multiple tables |
||
| 542 | * @param bool $fastFetch Whether to perform one query to load all |
||
| 543 | * the model data instead of fetching them |
||
| 544 | * one by one (ignores cache) |
||
| 545 | * @return array |
||
| 546 | */ |
||
| 547 | 4 | public function getModels($fastFetch = false) |
|
| 576 | |||
| 577 | /** |
||
| 578 | * Count the results |
||
| 579 | * |
||
| 580 | * @return int |
||
| 581 | */ |
||
| 582 | 1 | public function count() |
|
| 595 | |||
| 596 | /** |
||
| 597 | * Count the number of pages that all the models could be separated into |
||
| 598 | */ |
||
| 599 | public function countPages() |
||
| 600 | { |
||
| 601 | return ceil($this->count() / $this->getResultsPerPage()); |
||
| 602 | } |
||
| 603 | |||
| 604 | /** |
||
| 605 | * Find if there is any result |
||
| 606 | * |
||
| 607 | * @return bool |
||
| 608 | */ |
||
| 609 | 1 | public function any() |
|
| 618 | |||
| 619 | /** |
||
| 620 | * Get the amount of results that are returned per page |
||
| 621 | * @return int |
||
| 622 | */ |
||
| 623 | public function getResultsPerPage() |
||
| 624 | { |
||
| 625 | return $this->resultsPerPage; |
||
| 626 | } |
||
| 627 | |||
| 628 | /** |
||
| 629 | * Select a column to perform opeations on |
||
| 630 | * |
||
| 631 | * This is identical to the `where()` method, except that the column is |
||
| 632 | * specified as a MySQL column and not as a column name given by the model |
||
| 633 | * |
||
| 634 | * @param string $column The column to select |
||
| 635 | * @return static |
||
| 636 | */ |
||
| 637 | 4 | protected function column($column) |
|
| 652 | |||
| 653 | /** |
||
| 654 | * Add a condition for the column |
||
| 655 | * @param string $condition The MySQL condition |
||
| 656 | * @param mixed $value Value(s) to pass to MySQL |
||
| 657 | * @return void |
||
| 658 | */ |
||
| 659 | 4 | protected function addColumnCondition($condition, $value) |
|
| 675 | |||
| 676 | /** |
||
| 677 | * Get the MySQL extra parameters |
||
| 678 | * |
||
| 679 | * @param bool $respectPagination Whether to respect pagination or not; useful for when pagination should be ignored such as count |
||
| 680 | * @return string |
||
| 681 | */ |
||
| 682 | 4 | protected function createQueryParams($respectPagination = true) |
|
| 696 | |||
| 697 | /** |
||
| 698 | * Get the query parameters |
||
| 699 | * |
||
| 700 | * @return array |
||
| 701 | */ |
||
| 702 | 4 | protected function getParameters() |
|
| 706 | |||
| 707 | /** |
||
| 708 | * Get the table of the model |
||
| 709 | * |
||
| 710 | * @return string |
||
| 711 | */ |
||
| 712 | 4 | protected function getTable() |
|
| 718 | |||
| 719 | /** |
||
| 720 | * Get a MySQL query string in the requested format |
||
| 721 | * @param string|string[] $columns The columns that should be included |
||
| 722 | * (without the ID, if an array is provided) |
||
| 723 | * @return string The query |
||
| 724 | */ |
||
| 725 | 4 | protected function createQuery($columns = array()) |
|
| 739 | |||
| 740 | /** |
||
| 741 | * Generate the columns for the query |
||
| 742 | * @param string[] $columns The columns that should be included (without the ID) |
||
| 743 | * @return string |
||
| 744 | */ |
||
| 745 | 3 | private function createQueryColumns($columns = array()) |
|
| 763 | |||
| 764 | /** |
||
| 765 | * Generates all the WHERE conditions for the query |
||
| 766 | * @return string |
||
| 767 | */ |
||
| 768 | 4 | private function createQueryConditions() |
|
| 780 | |||
| 781 | /** |
||
| 782 | * Generates the sorting instructions for the query |
||
| 783 | * @return string |
||
| 784 | */ |
||
| 785 | 4 | private function createQueryOrder() |
|
| 786 | { |
||
| 787 | 4 | if ($this->sortBy) { |
|
| 788 | 2 | $order = 'ORDER BY ' . $this->sortBy; |
|
| 789 | |||
| 790 | // Sort by ID if the sorting columns are equal |
||
| 791 | 2 | $id = '`' . ((isset($this->tableAlias)) ? $this->tableAlias : $this->getTable()) . '`.`id`'; |
|
| 792 | 2 | if ($this->reverseSort) { |
|
| 793 | 2 | $order .= " DESC, $id DESC"; |
|
| 794 | } else { |
||
| 795 | 2 | $order .= ", $id"; |
|
| 796 | } |
||
| 797 | } else { |
||
| 798 | 3 | $order = ''; |
|
| 799 | } |
||
| 800 | |||
| 801 | 4 | return $order; |
|
| 802 | } |
||
| 803 | |||
| 804 | /** |
||
| 805 | * Generates the pagination instructions for the query |
||
| 806 | * @return string |
||
| 807 | */ |
||
| 808 | 4 | private function createQueryPagination() |
|
| 830 | } |
||
| 831 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.