Complex classes like Select 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 Select, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | class Select extends AbstractQuery |
||
16 | { |
||
17 | |||
18 | /** |
||
19 | * @param $name |
||
20 | * @param $arguments |
||
21 | * @return AbstractQuery|Select |
||
22 | */ |
||
23 | 8 | public function __call($name, $arguments) |
|
43 | |||
44 | /** |
||
45 | * Inserts FULLTEXT statement into $this->select and $this->where |
||
46 | * |
||
47 | * @param mixed $fields |
||
48 | * @param string $against |
||
49 | * @param string $alias |
||
50 | * @param boolean $boolean_mode |
||
51 | * @return $this |
||
52 | */ |
||
53 | public function match($fields, $against, $alias, $boolean_mode = true) |
||
75 | |||
76 | /** |
||
77 | * Inserts JOIN entry for the last table inserted by $this->from() |
||
78 | * |
||
79 | * @param mixed $table the table to be joined, given as simple string or name - alias pair |
||
80 | * @param string|boolean $on |
||
81 | * @param string $type SQL join type (INNER, OUTER, LEFT INNER, etc.) |
||
82 | * @return $this |
||
83 | */ |
||
84 | 2 | public function join($table, $on = false, $type = '') |
|
100 | |||
101 | /** |
||
102 | * Sets the group paramater for the query |
||
103 | * |
||
104 | * @param array $fields |
||
105 | * @param boolean $rollup suport for modifier WITH ROLLUP |
||
106 | * @return $this |
||
107 | */ |
||
108 | public function group($fields, $rollup = false) |
||
115 | |||
116 | /** |
||
117 | * @return string |
||
118 | */ |
||
119 | 7 | public function assemble() |
|
162 | |||
163 | /** |
||
164 | * @return null|string |
||
165 | */ |
||
166 | 7 | public function parseOptions() |
|
174 | |||
175 | /** |
||
176 | * @param $query |
||
177 | * @return Union |
||
178 | */ |
||
179 | public function union($query) |
||
180 | { |
||
181 | return new Union($this, $query); |
||
182 | } |
||
183 | |||
184 | /** |
||
185 | * Parses SELECT entries |
||
186 | * |
||
187 | * @return string |
||
188 | */ |
||
189 | 7 | protected function parseCols() |
|
211 | |||
212 | /** |
||
213 | * Parses FROM entries |
||
214 | * @return string |
||
215 | */ |
||
216 | 7 | private function parseFrom() |
|
246 | |||
247 | /** |
||
248 | * Parses JOIN entries for a given table |
||
249 | * Concatenates $this->join entries for input table |
||
250 | * |
||
251 | * @param string $table table to build JOIN statement for |
||
252 | * @return string |
||
253 | */ |
||
254 | 2 | private function parseJoin($table) |
|
255 | { |
||
256 | 2 | $result = ''; |
|
257 | |||
258 | 2 | if (isset($this->parts['join'][$table])) { |
|
259 | 2 | foreach ($this->parts['join'][$table] as $join) { |
|
260 | 2 | if (!is_array($join[0])) { |
|
261 | 1 | $join[0] = [$join[0]]; |
|
262 | } |
||
263 | |||
264 | 2 | $joinTable = isset($join[0][0]) ? $join[0][0] : false; |
|
265 | 2 | $joinAlias = isset($join[0][1]) ? $join[0][1] : false; |
|
266 | 2 | $joinOn = isset($join[1]) ? $join[1] : false; |
|
267 | |||
268 | |||
269 | 2 | $joinType = isset($join[2]) ? $join[2] : ''; |
|
270 | |||
271 | 2 | $result .= ($joinType ? ' '.strtoupper($joinType) : '').' JOIN '; |
|
272 | 2 | if ($joinTable instanceof AbstractQuery) { |
|
273 | $result .= '('.$joinTable.')'; |
||
274 | if (empty($joinAlias)) { |
||
275 | $joinAlias = 'join1'; |
||
276 | } |
||
277 | $joinTable = $joinAlias; |
||
278 | 2 | } elseif (strpos($joinTable, '(') !== false) { |
|
279 | $result .= $joinTable; |
||
280 | } else { |
||
281 | 2 | $result .= $this->protect($joinTable); |
|
282 | } |
||
283 | 2 | $result .= (!empty($joinAlias) ? ' AS '.$this->protect($joinAlias) : ''); |
|
284 | |||
285 | 2 | if ($joinOn) { |
|
286 | 2 | $result .= ' ON '; |
|
287 | 2 | if (is_array($joinOn)) { |
|
288 | 2 | $result .= $this->protect($table.'.'.$joinOn[0]) |
|
289 | 2 | .' = ' |
|
290 | 2 | .$this->protect($joinTable.'.'.$joinOn[1]); |
|
291 | } else { |
||
292 | 2 | $result .= '('.$joinOn.')'; |
|
293 | } |
||
294 | } |
||
295 | } |
||
296 | } |
||
297 | |||
298 | 2 | return $result; |
|
299 | } |
||
300 | |||
301 | /** |
||
302 | * Parses GROUP entries |
||
303 | * |
||
304 | * @uses $this->group['fields'] array with elements to group by |
||
305 | * @return string |
||
306 | */ |
||
307 | 7 | private function parseGroup() |
|
333 | } |
||
334 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: