sminnee /
silverstripe-framework
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 DataQuery 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 DataQuery, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 21 | class DataQuery |
||
| 22 | { |
||
| 23 | |||
| 24 | /** |
||
| 25 | * @var string |
||
| 26 | */ |
||
| 27 | protected $dataClass; |
||
| 28 | |||
| 29 | /** |
||
| 30 | * @var SQLSelect |
||
| 31 | */ |
||
| 32 | protected $query; |
||
| 33 | |||
| 34 | /** |
||
| 35 | * Map of all field names to an array of conflicting column SQL |
||
| 36 | * |
||
| 37 | * E.g. |
||
| 38 | * array( |
||
| 39 | * 'Title' => array( |
||
| 40 | * '"MyTable"."Title"', |
||
| 41 | * '"AnotherTable"."Title"', |
||
| 42 | * ) |
||
| 43 | * ) |
||
| 44 | * |
||
| 45 | * @var array |
||
| 46 | */ |
||
| 47 | protected $collidingFields = array(); |
||
| 48 | |||
| 49 | /** |
||
| 50 | * Allows custom callback to be registered before getFinalisedQuery is called. |
||
| 51 | * |
||
| 52 | * @var DataQueryManipulator[] |
||
| 53 | */ |
||
| 54 | protected $dataQueryManipulators = []; |
||
| 55 | |||
| 56 | private $queriedColumns = null; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var bool |
||
| 60 | */ |
||
| 61 | private $queryFinalised = false; |
||
| 62 | |||
| 63 | // TODO: replace subclass_access with this |
||
| 64 | protected $querySubclasses = true; |
||
| 65 | // TODO: replace restrictclasses with this |
||
| 66 | protected $filterByClassName = true; |
||
| 67 | |||
| 68 | /** |
||
| 69 | * Create a new DataQuery. |
||
| 70 | * |
||
| 71 | * @param string $dataClass The name of the DataObject class that you wish to query |
||
| 72 | */ |
||
| 73 | public function __construct($dataClass) |
||
| 78 | |||
| 79 | /** |
||
| 80 | * Clone this object |
||
| 81 | */ |
||
| 82 | public function __clone() |
||
| 86 | |||
| 87 | /** |
||
| 88 | * Return the {@link DataObject} class that is being queried. |
||
| 89 | * |
||
| 90 | * @return string |
||
| 91 | */ |
||
| 92 | public function dataClass() |
||
| 96 | |||
| 97 | /** |
||
| 98 | * Return the {@link SQLSelect} object that represents the current query; note that it will |
||
| 99 | * be a clone of the object. |
||
| 100 | * |
||
| 101 | * @return SQLSelect |
||
| 102 | */ |
||
| 103 | public function query() |
||
| 107 | |||
| 108 | |||
| 109 | /** |
||
| 110 | * Remove a filter from the query |
||
| 111 | * |
||
| 112 | * @param string|array $fieldExpression The predicate of the condition to remove |
||
| 113 | * (ignoring parameters). The expression will be considered a match if it's |
||
| 114 | * contained within any other predicate. |
||
| 115 | * @return DataQuery Self reference |
||
| 116 | */ |
||
| 117 | public function removeFilterOn($fieldExpression) |
||
| 158 | |||
| 159 | /** |
||
| 160 | * Set up the simplest initial query |
||
| 161 | */ |
||
| 162 | protected function initialiseQuery() |
||
| 184 | |||
| 185 | public function setQueriedColumns($queriedColumns) |
||
| 189 | |||
| 190 | /** |
||
| 191 | * Ensure that the query is ready to execute. |
||
| 192 | * |
||
| 193 | * @param array|null $queriedColumns Any columns to filter the query by |
||
| 194 | * @return SQLSelect The finalised sql query |
||
| 195 | */ |
||
| 196 | public function getFinalisedQuery($queriedColumns = null) |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Ensure that if a query has an order by clause, those columns are present in the select. |
||
| 344 | * |
||
| 345 | * @param SQLSelect $query |
||
| 346 | * @param array $originalSelect |
||
| 347 | */ |
||
| 348 | protected function ensureSelectContainsOrderbyColumns($query, $originalSelect = array()) |
||
| 411 | |||
| 412 | /** |
||
| 413 | * Execute the query and return the result as {@link SS_Query} object. |
||
| 414 | * |
||
| 415 | * @return Query |
||
| 416 | */ |
||
| 417 | public function execute() |
||
| 421 | |||
| 422 | /** |
||
| 423 | * Return this query's SQL |
||
| 424 | * |
||
| 425 | * @param array $parameters Out variable for parameters required for this query |
||
| 426 | * @return string The resulting SQL query (may be paramaterised) |
||
| 427 | */ |
||
| 428 | public function sql(&$parameters = array()) |
||
| 432 | |||
| 433 | /** |
||
| 434 | * Return the number of records in this query. |
||
| 435 | * Note that this will issue a separate SELECT COUNT() query. |
||
| 436 | * |
||
| 437 | * @return int |
||
| 438 | */ |
||
| 439 | public function count() |
||
| 444 | |||
| 445 | /** |
||
| 446 | * Return the maximum value of the given field in this DataList |
||
| 447 | * |
||
| 448 | * @param String $field Unquoted database column name. Will be ANSI quoted |
||
| 449 | * automatically so must not contain double quotes. |
||
| 450 | * @return string |
||
| 451 | */ |
||
| 452 | View Code Duplication | public function max($field) |
|
| 460 | |||
| 461 | /** |
||
| 462 | * Return the minimum value of the given field in this DataList |
||
| 463 | * |
||
| 464 | * @param string $field Unquoted database column name. Will be ANSI quoted |
||
| 465 | * automatically so must not contain double quotes. |
||
| 466 | * @return string |
||
| 467 | */ |
||
| 468 | View Code Duplication | public function min($field) |
|
| 476 | |||
| 477 | /** |
||
| 478 | * Return the average value of the given field in this DataList |
||
| 479 | * |
||
| 480 | * @param string $field Unquoted database column name. Will be ANSI quoted |
||
| 481 | * automatically so must not contain double quotes. |
||
| 482 | * @return string |
||
| 483 | */ |
||
| 484 | View Code Duplication | public function avg($field) |
|
| 492 | |||
| 493 | /** |
||
| 494 | * Return the sum of the values of the given field in this DataList |
||
| 495 | * |
||
| 496 | * @param string $field Unquoted database column name. Will be ANSI quoted |
||
| 497 | * automatically so must not contain double quotes. |
||
| 498 | * @return string |
||
| 499 | */ |
||
| 500 | View Code Duplication | public function sum($field) |
|
| 508 | |||
| 509 | /** |
||
| 510 | * Runs a raw aggregate expression. Please handle escaping yourself |
||
| 511 | * |
||
| 512 | * @param string $expression An aggregate expression, such as 'MAX("Balance")', or a set of them |
||
| 513 | * (as an escaped SQL statement) |
||
| 514 | * @return string |
||
| 515 | */ |
||
| 516 | public function aggregate($expression) |
||
| 520 | |||
| 521 | /** |
||
| 522 | * Return the first row that would be returned by this full DataQuery |
||
| 523 | * Note that this will issue a separate SELECT ... LIMIT 1 query. |
||
| 524 | */ |
||
| 525 | public function firstRow() |
||
| 529 | |||
| 530 | /** |
||
| 531 | * Return the last row that would be returned by this full DataQuery |
||
| 532 | * Note that this will issue a separate SELECT ... LIMIT query. |
||
| 533 | */ |
||
| 534 | public function lastRow() |
||
| 538 | |||
| 539 | /** |
||
| 540 | * Update the SELECT clause of the query with the columns from the given table |
||
| 541 | * |
||
| 542 | * @param SQLSelect $query |
||
| 543 | * @param string $tableClass Class to select from |
||
| 544 | * @param array $columns |
||
| 545 | */ |
||
| 546 | protected function selectColumnsFromTable(SQLSelect &$query, $tableClass, $columns = null) |
||
| 577 | |||
| 578 | /** |
||
| 579 | * Append a GROUP BY clause to this query. |
||
| 580 | * |
||
| 581 | * @param string $groupby Escaped SQL statement |
||
| 582 | * @return $this |
||
| 583 | */ |
||
| 584 | public function groupby($groupby) |
||
| 589 | |||
| 590 | /** |
||
| 591 | * Append a HAVING clause to this query. |
||
| 592 | * |
||
| 593 | * @param string $having Escaped SQL statement |
||
| 594 | * @return $this |
||
| 595 | */ |
||
| 596 | public function having($having) |
||
| 601 | |||
| 602 | /** |
||
| 603 | * Create a disjunctive subgroup. |
||
| 604 | * |
||
| 605 | * That is a subgroup joined by OR |
||
| 606 | * |
||
| 607 | * @return DataQuery_SubGroup |
||
| 608 | */ |
||
| 609 | public function disjunctiveGroup() |
||
| 613 | |||
| 614 | /** |
||
| 615 | * Create a conjunctive subgroup |
||
| 616 | * |
||
| 617 | * That is a subgroup joined by AND |
||
| 618 | * |
||
| 619 | * @return DataQuery_SubGroup |
||
| 620 | */ |
||
| 621 | public function conjunctiveGroup() |
||
| 625 | |||
| 626 | /** |
||
| 627 | * Adds a WHERE clause. |
||
| 628 | * |
||
| 629 | * @see SQLSelect::addWhere() for syntax examples, although DataQuery |
||
| 630 | * won't expand multiple arguments as SQLSelect does. |
||
| 631 | * |
||
| 632 | * @param string|array|SQLConditionGroup $filter Predicate(s) to set, as escaped SQL statements or |
||
| 633 | * paramaterised queries |
||
| 634 | * @return DataQuery |
||
| 635 | */ |
||
| 636 | public function where($filter) |
||
| 643 | |||
| 644 | /** |
||
| 645 | * Append a WHERE with OR. |
||
| 646 | * |
||
| 647 | * @see SQLSelect::addWhere() for syntax examples, although DataQuery |
||
| 648 | * won't expand multiple method arguments as SQLSelect does. |
||
| 649 | * |
||
| 650 | * @param string|array|SQLConditionGroup $filter Predicate(s) to set, as escaped SQL statements or |
||
| 651 | * paramaterised queries |
||
| 652 | * @return DataQuery |
||
| 653 | */ |
||
| 654 | public function whereAny($filter) |
||
| 661 | |||
| 662 | /** |
||
| 663 | * Set the ORDER BY clause of this query |
||
| 664 | * |
||
| 665 | * @see SQLSelect::orderby() |
||
| 666 | * |
||
| 667 | * @param String $sort Column to sort on (escaped SQL statement) |
||
| 668 | * @param String $direction Direction ("ASC" or "DESC", escaped SQL statement) |
||
| 669 | * @param Boolean $clear Clear existing values |
||
| 670 | * @return DataQuery |
||
| 671 | */ |
||
| 672 | public function sort($sort = null, $direction = null, $clear = true) |
||
| 682 | |||
| 683 | /** |
||
| 684 | * Reverse order by clause |
||
| 685 | * |
||
| 686 | * @return DataQuery |
||
| 687 | */ |
||
| 688 | public function reverseSort() |
||
| 693 | |||
| 694 | /** |
||
| 695 | * Set the limit of this query. |
||
| 696 | * |
||
| 697 | * @param int $limit |
||
| 698 | * @param int $offset |
||
| 699 | * @return $this |
||
| 700 | */ |
||
| 701 | public function limit($limit, $offset = 0) |
||
| 706 | |||
| 707 | /** |
||
| 708 | * Set whether this query should be distinct or not. |
||
| 709 | * |
||
| 710 | * @param bool $value |
||
| 711 | * @return DataQuery |
||
| 712 | */ |
||
| 713 | public function distinct($value) |
||
| 718 | |||
| 719 | /** |
||
| 720 | * Add an INNER JOIN clause to this query. |
||
| 721 | * |
||
| 722 | * @param String $table The unquoted table name to join to. |
||
| 723 | * @param String $onClause The filter for the join (escaped SQL statement) |
||
| 724 | * @param String $alias An optional alias name (unquoted) |
||
| 725 | * @param int $order A numerical index to control the order that joins are added to the query; lower order values |
||
| 726 | * will cause the query to appear first. The default is 20, and joins created automatically by the |
||
| 727 | * ORM have a value of 10. |
||
| 728 | * @param array $parameters Any additional parameters if the join is a parameterised subquery |
||
| 729 | * @return $this |
||
| 730 | */ |
||
| 731 | View Code Duplication | public function innerJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) |
|
| 738 | |||
| 739 | /** |
||
| 740 | * Add a LEFT JOIN clause to this query. |
||
| 741 | * |
||
| 742 | * @param string $table The unquoted table to join to. |
||
| 743 | * @param string $onClause The filter for the join (escaped SQL statement). |
||
| 744 | * @param string $alias An optional alias name (unquoted) |
||
| 745 | * @param int $order A numerical index to control the order that joins are added to the query; lower order values |
||
| 746 | * will cause the query to appear first. The default is 20, and joins created automatically by the |
||
| 747 | * ORM have a value of 10. |
||
| 748 | * @param array $parameters Any additional parameters if the join is a parameterised subquery |
||
| 749 | * @return $this |
||
| 750 | */ |
||
| 751 | View Code Duplication | public function leftJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) |
|
| 758 | |||
| 759 | /** |
||
| 760 | * Prefix of all joined table aliases. E.g. ->filter('Banner.Image.Title)' |
||
| 761 | * Will join the Banner, and then Image relations |
||
| 762 | * `$relationPrefx` will be `banner_image_` |
||
| 763 | * Each table in the Image chain will be suffixed to this prefix. E.g. |
||
| 764 | * `banner_image_File` and `banner_image_Image` |
||
| 765 | * |
||
| 766 | * This will be null if no relation is joined. |
||
| 767 | * E.g. `->filter('Title')` |
||
| 768 | * |
||
| 769 | * @param string|array $relation Relation in '.' delimited string, or array of parts |
||
| 770 | * @return string Table prefix |
||
| 771 | */ |
||
| 772 | public static function applyRelationPrefix($relation) |
||
| 782 | |||
| 783 | /** |
||
| 784 | * Traverse the relationship fields, and add the table |
||
| 785 | * mappings to the query object state. This has to be called |
||
| 786 | * in any overloaded {@link SearchFilter->apply()} methods manually. |
||
| 787 | * |
||
| 788 | * Note, that in order to filter against the joined relation user code must |
||
| 789 | * use {@see tablePrefix()} to get the table alias used for this relation. |
||
| 790 | * |
||
| 791 | * @param string|array $relation The array/dot-syntax relation to follow |
||
| 792 | * @param bool $linearOnly Set to true to restrict to linear relations only. Set this |
||
| 793 | * if this relation will be used for sorting, and should not include duplicate rows. |
||
| 794 | * @return string The model class of the related item |
||
| 795 | */ |
||
| 796 | public function applyRelation($relation, $linearOnly = false) |
||
| 871 | |||
| 872 | /** |
||
| 873 | * Join the given has_many relation to this query. |
||
| 874 | * Also works with belongs_to |
||
| 875 | * |
||
| 876 | * Doesn't work with polymorphic relationships |
||
| 877 | * |
||
| 878 | * @param string $localClass Name of class that has the has_many to the joined class |
||
| 879 | * @param string $localField Name of the has_many relationship to join |
||
| 880 | * @param string $foreignClass Class to join |
||
| 881 | * @param string $localPrefix Table prefix for parent class |
||
| 882 | * @param string $foreignPrefix Table prefix to use |
||
| 883 | * @param string $type 'has_many' or 'belongs_to' |
||
| 884 | */ |
||
| 885 | protected function joinHasManyRelation( |
||
| 942 | |||
| 943 | /** |
||
| 944 | * Join the given class to this query with the given key |
||
| 945 | * |
||
| 946 | * @param string $localClass Name of class that has the has_one to the joined class |
||
| 947 | * @param string $localField Name of the has_one relationship to joi |
||
| 948 | * @param string $foreignClass Class to join |
||
| 949 | * @param string $localPrefix Table prefix to use for local class |
||
| 950 | * @param string $foreignPrefix Table prefix to use for joined table |
||
| 951 | */ |
||
| 952 | protected function joinHasOneRelation( |
||
| 1004 | |||
| 1005 | /** |
||
| 1006 | * Join table via many_many relationship |
||
| 1007 | * |
||
| 1008 | * @param string $relationClass |
||
| 1009 | * @param string $parentClass |
||
| 1010 | * @param string $componentClass |
||
| 1011 | * @param string $parentField |
||
| 1012 | * @param string $componentField |
||
| 1013 | * @param string $relationClassOrTable Name of relation table |
||
| 1014 | * @param string $parentPrefix Table prefix for parent class |
||
| 1015 | * @param string $componentPrefix Table prefix to use for both joined and mapping table |
||
| 1016 | */ |
||
| 1017 | protected function joinManyManyRelationship( |
||
| 1074 | |||
| 1075 | /** |
||
| 1076 | * Removes the result of query from this query. |
||
| 1077 | * |
||
| 1078 | * @param DataQuery $subtractQuery |
||
| 1079 | * @param string $field |
||
| 1080 | * @return $this |
||
| 1081 | */ |
||
| 1082 | public function subtract(DataQuery $subtractQuery, $field = 'ID') |
||
| 1094 | |||
| 1095 | /** |
||
| 1096 | * Select the only given fields from the given table. |
||
| 1097 | * |
||
| 1098 | * @param string $table Unquoted table name (will be escaped automatically) |
||
| 1099 | * @param array $fields Database column names (will be escaped automatically) |
||
| 1100 | * @return $this |
||
| 1101 | */ |
||
| 1102 | public function selectFromTable($table, $fields) |
||
| 1112 | |||
| 1113 | /** |
||
| 1114 | * Add the given fields from the given table to the select statement. |
||
| 1115 | * |
||
| 1116 | * @param string $table Unquoted table name (will be escaped automatically) |
||
| 1117 | * @param array $fields Database column names (will be escaped automatically) |
||
| 1118 | * @return $this |
||
| 1119 | */ |
||
| 1120 | public function addSelectFromTable($table, $fields) |
||
| 1130 | |||
| 1131 | /** |
||
| 1132 | * Query the given field column from the database and return as an array. |
||
| 1133 | * |
||
| 1134 | * @param string $field See {@link expressionForField()}. |
||
| 1135 | * @return array List of column values for the specified column |
||
| 1136 | */ |
||
| 1137 | public function column($field = 'ID') |
||
| 1148 | |||
| 1149 | /** |
||
| 1150 | * @param String $field Select statement identifier, either the unquoted column name, |
||
| 1151 | * the full composite SQL statement, or the alias set through {@link SQLSelect->selectField()}. |
||
| 1152 | * @return String The expression used to query this field via this DataQuery |
||
| 1153 | */ |
||
| 1154 | protected function expressionForField($field) |
||
| 1171 | |||
| 1172 | /** |
||
| 1173 | * Select the given field expressions. |
||
| 1174 | * |
||
| 1175 | * @param $fieldExpression String The field to select (escaped SQL statement) |
||
| 1176 | * @param $alias String The alias of that field (escaped SQL statement) |
||
| 1177 | */ |
||
| 1178 | public function selectField($fieldExpression, $alias = null) |
||
| 1182 | |||
| 1183 | //// QUERY PARAMS |
||
| 1184 | |||
| 1185 | /** |
||
| 1186 | * An arbitrary store of query parameters that can be used by decorators. |
||
| 1187 | */ |
||
| 1188 | private $queryParams; |
||
| 1189 | |||
| 1190 | /** |
||
| 1191 | * Set an arbitrary query parameter, that can be used by decorators to add additional meta-data to the query. |
||
| 1192 | * It's expected that the $key will be namespaced, e.g, 'Versioned.stage' instead of just 'stage'. |
||
| 1193 | * |
||
| 1194 | * @param string $key |
||
| 1195 | * @param string|array $value |
||
| 1196 | * @return $this |
||
| 1197 | */ |
||
| 1198 | public function setQueryParam($key, $value) |
||
| 1203 | |||
| 1204 | /** |
||
| 1205 | * Set an arbitrary query parameter, that can be used by decorators to add additional meta-data to the query. |
||
| 1206 | * |
||
| 1207 | * @param string $key |
||
| 1208 | * @return string |
||
| 1209 | */ |
||
| 1210 | public function getQueryParam($key) |
||
| 1217 | |||
| 1218 | /** |
||
| 1219 | * Returns all query parameters |
||
| 1220 | * @return array query parameters array |
||
| 1221 | */ |
||
| 1222 | public function getQueryParams() |
||
| 1226 | |||
| 1227 | /** |
||
| 1228 | * Get query manipulators |
||
| 1229 | * |
||
| 1230 | * @return DataQueryManipulator[] |
||
| 1231 | */ |
||
| 1232 | public function getDataQueryManipulators() |
||
| 1236 | |||
| 1237 | /** |
||
| 1238 | * Assign callback to be invoked in getFinalisedQuery() |
||
| 1239 | * |
||
| 1240 | * @param DataQueryManipulator $manipulator |
||
| 1241 | * @return $this |
||
| 1242 | */ |
||
| 1243 | public function pushQueryManipulator(DataQueryManipulator $manipulator) |
||
| 1248 | } |
||
| 1249 |
Loading history...
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)or! empty(...)instead.