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 SQLQueryBuilder 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 SQLQueryBuilder, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
5 | class SQLQueryBuilder extends AbstractQueryBuilder |
||
6 | { |
||
7 | |||
8 | private $queryBuilder; |
||
9 | |||
10 | 2 | public function run() |
|
11 | { |
||
12 | 2 | $this->queryBuilder = $this->getQueryBuilder(); |
|
13 | 2 | $this->setJoins(); |
|
14 | 2 | $numberOfRows = $this->getCount(); |
|
15 | 2 | if ($numberOfRows === 0) { |
|
16 | return ['total' => 0, 'data' => null]; |
||
17 | } |
||
18 | 2 | $this->setSortOrders(); |
|
19 | 2 | $this->setOffsetAndLimit(); |
|
20 | 2 | $this->setReturnFields(); |
|
21 | 2 | $stmt = $this->conn->executeQuery( |
|
22 | 2 | $this->queryBuilder->getSql(), |
|
23 | 2 | $this->queryBuilder->getParameters() |
|
24 | ); |
||
25 | 2 | $result = $stmt->fetchAll(\PDO::FETCH_ASSOC); |
|
26 | 2 | if ($this->distinctFieldName !== null) { |
|
27 | 1 | $numberOfRows = count($result); |
|
28 | } |
||
29 | 2 | return ['total' => $numberOfRows, 'data' => $result]; |
|
30 | } |
||
31 | |||
32 | 2 | private function getQueryBuilder() |
|
33 | { |
||
34 | 2 | if ($this->orFilters !== null) { |
|
35 | 2 | $this->andFilters[] = $this->orFilters; |
|
36 | } |
||
37 | 2 | $this->filters = $this->andFilters; |
|
38 | 2 | return $this->buildQuery($this->collection, $this->filters); |
|
39 | } |
||
40 | |||
41 | 2 | private function getCount() |
|
42 | { |
||
43 | 2 | $queryBuilderCount = clone $this->queryBuilder; |
|
44 | 2 | $queryBuilderCount->select(' COUNT(*) AS total '); |
|
45 | 2 | $stmt = $this->conn->executeQuery($queryBuilderCount->getSql(), $queryBuilderCount->getParameters()); |
|
46 | 2 | return (int) $stmt->fetch(\PDO::FETCH_ASSOC)['total']; |
|
47 | } |
||
48 | |||
49 | 2 | private function setSortOrders() |
|
50 | { |
||
51 | 2 | if ($this->sortFields !== null) { |
|
52 | foreach ($this->addAlias($this->sortFields) as $sortKey => $sortDir) { |
||
|
|||
53 | $this->queryBuilder->addOrderBy($sortKey, $sortDir); |
||
54 | } |
||
55 | } |
||
56 | 2 | } |
|
57 | |||
58 | 2 | private function setJoins() |
|
59 | { |
||
60 | 2 | $this->setJoinsForType('innerJoin'); |
|
61 | 2 | $this->setJoinsForType('leftJoin'); |
|
62 | 2 | $this->setJoinsForType('rightJoin'); |
|
63 | 2 | $this->setJoinsForType('outerJoin'); |
|
64 | 2 | } |
|
65 | |||
66 | 2 | private function setJoinsForType($joinType) |
|
67 | { |
||
68 | 2 | if ($this->{$joinType} === null) { |
|
69 | 2 | return; |
|
70 | } |
||
71 | 1 | foreach ($this->{$joinType} as $collectionName => $collection) { |
|
72 | 1 | $fieldNames = $this->addAlias($collection['returnFields'], $collectionName); |
|
73 | 1 | $this->returnFieldsForJoin($fieldNames); |
|
74 | 1 | $joinCondition = ''; |
|
75 | 1 | foreach ($collection['relations'] as $relation) { |
|
76 | 1 | $joinCondition .= empty($joinCondition) ? '':' AND '; |
|
77 | 1 | $relationType = array_keys($relation)[0]; |
|
78 | 1 | $source = array_keys($relation[$relationType])[0]; |
|
79 | 1 | $condition = $collectionName . '.' . $source |
|
80 | 1 | . ' = ' . $this->collection . '.' . $relation[$relationType][$source]; |
|
81 | 1 | if ($relationType != 'field') { |
|
82 | 1 | $condition = $collectionName . '.' . $source . ' ' |
|
83 | 1 | . $relationType . ' '. $relation[$relationType][$source]; |
|
84 | } |
||
85 | 1 | $joinCondition .= $condition; |
|
86 | } |
||
87 | 1 | $this->queryBuilder->{$joinType}($this->collection, $collectionName, $collectionName, $joinCondition); |
|
88 | } |
||
89 | 1 | return $this; |
|
90 | } |
||
91 | 1 | public function returnFieldsForJoin(array $fieldNames = null) |
|
92 | { |
||
93 | 1 | if ($fieldNames !== null) { |
|
94 | 1 | foreach ($fieldNames as $fieldName) { |
|
95 | 1 | $this->fieldNames[] = $fieldName; |
|
96 | } |
||
97 | } |
||
98 | 1 | } |
|
99 | |||
100 | 2 | private function setReturnFields() |
|
101 | { |
||
102 | 2 | if ($this->distinctFieldName === null) { |
|
103 | 2 | $fieldNames = ($this->fieldNames === null) |
|
104 | 1 | ? $this->addAlias('*') |
|
105 | 2 | : $this->addAlias($this->fieldNames); |
|
106 | 2 | $this->queryBuilder->select($fieldNames); |
|
107 | 2 | return; |
|
108 | } |
||
109 | 1 | $this->queryBuilder->select('DISTINCT (' . $this->collection . '.' . $this->distinctFieldName . ')'); |
|
110 | 1 | } |
|
111 | |||
112 | 2 | private function setOffsetAndLimit() |
|
117 | |||
118 | 2 | private function addAlias($fields, $collection = null) |
|
119 | { |
||
120 | 2 | $collection = (null !== $collection) ? $collection : $this->collection; |
|
140 | |||
141 | 2 | protected function buildQuery($collection, $filters) |
|
150 | |||
151 | 2 | protected function buildQueryFilters($queryBuilder, $filters) |
|
162 | |||
163 | 2 | protected function buildQueryForAnd($queryBuilder, $key, $value) |
|
204 | |||
205 | |||
206 | 2 | public static function buildFilter($filter) |
|
245 | } |
||
246 |
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.