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 BaseQuery 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 BaseQuery, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | abstract class BaseQuery |
||
13 | { |
||
14 | /** |
||
15 | * Bitrix object to be queried. |
||
16 | * |
||
17 | * @var object |
||
18 | */ |
||
19 | protected $bxObject; |
||
20 | |||
21 | /** |
||
22 | * Name of the model that calls the query. |
||
23 | * |
||
24 | * @var string |
||
25 | */ |
||
26 | protected $modelName; |
||
27 | |||
28 | /** |
||
29 | * Model that calls the query. |
||
30 | * |
||
31 | * @var object |
||
32 | */ |
||
33 | protected $model; |
||
34 | |||
35 | /** |
||
36 | * Query sort. |
||
37 | * |
||
38 | * @var array |
||
39 | */ |
||
40 | public $sort = []; |
||
41 | |||
42 | /** |
||
43 | * Query filter. |
||
44 | * |
||
45 | * @var array |
||
46 | */ |
||
47 | public $filter = []; |
||
48 | |||
49 | /** |
||
50 | * Query navigation. |
||
51 | * |
||
52 | * @var array|bool |
||
53 | */ |
||
54 | public $navigation = false; |
||
55 | |||
56 | /** |
||
57 | * Query select. |
||
58 | * |
||
59 | * @var array |
||
60 | */ |
||
61 | public $select = ['FIELDS', 'PROPS']; |
||
62 | |||
63 | /** |
||
64 | * The key to list items in array of results. |
||
65 | * Set to false to have auto incrementing integer. |
||
66 | * |
||
67 | * @var string|bool |
||
68 | */ |
||
69 | public $keyBy = 'ID'; |
||
70 | |||
71 | /** |
||
72 | * Indicates that the query should be stopped instead of touching the DB. |
||
73 | * Can be set in query scopes or manually. |
||
74 | * |
||
75 | * @var bool |
||
76 | */ |
||
77 | protected $queryShouldBeStopped = false; |
||
78 | |||
79 | /** |
||
80 | * Get count of users that match $filter. |
||
81 | * |
||
82 | * @return int |
||
83 | */ |
||
84 | abstract public function count(); |
||
85 | |||
86 | /** |
||
87 | * Get list of items. |
||
88 | * |
||
89 | * @return Collection |
||
90 | */ |
||
91 | abstract public function getList(); |
||
92 | |||
93 | /** |
||
94 | * Constructor. |
||
95 | * |
||
96 | * @param object $bxObject |
||
97 | * @param string $modelName |
||
98 | */ |
||
99 | public function __construct($bxObject, $modelName) |
||
105 | |||
106 | /** |
||
107 | * Get the first item that matches query params. |
||
108 | * |
||
109 | * @return mixed |
||
110 | */ |
||
111 | public function first() |
||
115 | |||
116 | /** |
||
117 | * Get item by its id. |
||
118 | * |
||
119 | * @param int $id |
||
120 | * |
||
121 | * @return mixed |
||
122 | */ |
||
123 | public function getById($id) |
||
134 | |||
135 | /** |
||
136 | * Setter for sort. |
||
137 | * |
||
138 | * @param mixed $by |
||
139 | * @param string $order |
||
140 | * |
||
141 | * @return $this |
||
142 | */ |
||
143 | public function sort($by, $order = 'ASC') |
||
149 | |||
150 | /** |
||
151 | * Setter for filter. |
||
152 | * |
||
153 | * @param array $filter |
||
154 | * |
||
155 | * @return $this |
||
156 | */ |
||
157 | public function filter($filter) |
||
163 | |||
164 | /** |
||
165 | * Reset filter. |
||
166 | * |
||
167 | * @return $this |
||
168 | */ |
||
169 | public function resetFilter() |
||
175 | |||
176 | /** |
||
177 | * Add another filter to filters array. |
||
178 | * |
||
179 | * @param array $filters |
||
180 | * |
||
181 | * @return $this |
||
182 | */ |
||
183 | public function addFilter($filters) |
||
191 | |||
192 | /** |
||
193 | * Setter for navigation. |
||
194 | * |
||
195 | * @param $value |
||
196 | * |
||
197 | * @return $this |
||
198 | */ |
||
199 | public function navigation($value) |
||
205 | |||
206 | /** |
||
207 | * Setter for select. |
||
208 | * |
||
209 | * @param $value |
||
210 | * |
||
211 | * @return $this |
||
212 | */ |
||
213 | public function select($value) |
||
219 | |||
220 | /** |
||
221 | * Setter for keyBy. |
||
222 | * |
||
223 | * @param string $value |
||
224 | * |
||
225 | * @return $this |
||
226 | */ |
||
227 | public function keyBy($value) |
||
233 | |||
234 | /** |
||
235 | * Set the "limit" value of the query. |
||
236 | * |
||
237 | * @param int $value |
||
238 | * |
||
239 | * @return $this |
||
240 | */ |
||
241 | public function limit($value) |
||
247 | |||
248 | /** |
||
249 | * Set the "page number" value of the query. |
||
250 | * |
||
251 | * @param int $num |
||
252 | * |
||
253 | * @return $this |
||
254 | */ |
||
255 | public function page($num) |
||
261 | |||
262 | /** |
||
263 | * Alias for "limit". |
||
264 | * |
||
265 | * @param int $value |
||
266 | * |
||
267 | * @return $this |
||
268 | */ |
||
269 | public function take($value) |
||
273 | |||
274 | /** |
||
275 | * Set the limit and offset for a given page. |
||
276 | * |
||
277 | * @param int $page |
||
278 | * @param int $perPage |
||
279 | * @return $this |
||
280 | */ |
||
281 | public function forPage($page, $perPage = 15) |
||
285 | |||
286 | /** |
||
287 | * Paginate the given query into a paginator. |
||
288 | * |
||
289 | * @param int $perPage |
||
290 | * @param string $pageName |
||
291 | * |
||
292 | * @return \Illuminate\Pagination\LengthAwarePaginator |
||
293 | */ |
||
294 | View Code Duplication | public function paginate($perPage = 15, $pageName = 'page') |
|
305 | |||
306 | /** |
||
307 | * Get a paginator only supporting simple next and previous links. |
||
308 | * |
||
309 | * This is more efficient on larger data-sets, etc. |
||
310 | * |
||
311 | * @param int $perPage |
||
312 | * @param string $pageName |
||
313 | * |
||
314 | * @return \Illuminate\Pagination\Paginator |
||
315 | */ |
||
316 | View Code Duplication | public function simplePaginate($perPage = 15, $pageName = 'page') |
|
326 | |||
327 | /** |
||
328 | * Stop the query from touching DB. |
||
329 | * |
||
330 | * @return $this |
||
331 | */ |
||
332 | public function stopQuery() |
||
338 | |||
339 | /** |
||
340 | * Adds $item to $results using keyBy value. |
||
341 | * |
||
342 | * @param $results |
||
343 | * @param BitrixModel $object |
||
344 | * |
||
345 | * @return void |
||
346 | */ |
||
347 | protected function addItemToResultsUsingKeyBy(&$results, BitrixModel $object) |
||
389 | |||
390 | /** |
||
391 | * Determine if all fields must be selected. |
||
392 | * |
||
393 | * @return bool |
||
394 | */ |
||
395 | protected function fieldsMustBeSelected() |
||
399 | |||
400 | /** |
||
401 | * Determine if all fields must be selected. |
||
402 | * |
||
403 | * @return bool |
||
404 | */ |
||
405 | protected function propsMustBeSelected() |
||
411 | |||
412 | /** |
||
413 | * Set $array[$new] as $array[$old] and delete $array[$old]. |
||
414 | * |
||
415 | * @param array $array |
||
416 | * @param $old |
||
417 | * @param $new |
||
418 | * |
||
419 | * return null |
||
420 | */ |
||
421 | protected function substituteField(&$array, $old, $new) |
||
429 | |||
430 | /** |
||
431 | * Clear select array from duplication and additional fields. |
||
432 | * |
||
433 | * @return array |
||
434 | */ |
||
435 | protected function clearSelectArray() |
||
441 | |||
442 | /** |
||
443 | * Handle dynamic method calls into the method. |
||
444 | * |
||
445 | * @param string $method |
||
446 | * @param array $parameters |
||
447 | * |
||
448 | * @throws BadMethodCallException |
||
449 | * |
||
450 | * @return $this |
||
451 | */ |
||
452 | public function __call($method, $parameters) |
||
470 | } |
||
471 |
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.