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 Client 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 Client, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class Client implements Interfaces\ClientInterface |
||
17 | { |
||
18 | use SocketTrait, ShortsTrait; |
||
19 | |||
20 | /** |
||
21 | * Configuration of connection |
||
22 | * |
||
23 | * @var \RouterOS\Config |
||
24 | */ |
||
25 | private $_config; |
||
26 | |||
27 | /** |
||
28 | * API communication object |
||
29 | * |
||
30 | * @var \RouterOS\APIConnector |
||
31 | */ |
||
32 | |||
33 | private $_connector; |
||
34 | |||
35 | /** |
||
36 | * Client constructor. |
||
37 | * |
||
38 | * @param array|\RouterOS\Config $config |
||
39 | * @throws \RouterOS\Exceptions\ClientException |
||
40 | * @throws \RouterOS\Exceptions\ConfigException |
||
41 | * @throws \RouterOS\Exceptions\QueryException |
||
42 | */ |
||
43 | 17 | public function __construct($config) |
|
63 | |||
64 | /** |
||
65 | * Get some parameter from config |
||
66 | * |
||
67 | * @param string $parameter Name of required parameter |
||
68 | * @return mixed |
||
69 | * @throws \RouterOS\Exceptions\ConfigException |
||
70 | */ |
||
71 | 16 | private function config(string $parameter) |
|
75 | |||
76 | /** |
||
77 | * Send write query to RouterOS |
||
78 | * |
||
79 | * @param string|array|\RouterOS\Query $query |
||
80 | * @return \RouterOS\Client |
||
81 | * @throws \RouterOS\Exceptions\QueryException |
||
82 | * @deprecated |
||
83 | * @codeCoverageIgnore |
||
84 | */ |
||
85 | public function write($query): Client |
||
101 | |||
102 | /** |
||
103 | * Send write query to RouterOS (modern version of write) |
||
104 | * |
||
105 | * @param string $endpoint Path of API query |
||
106 | * @param array|null $where List of where filters |
||
107 | * @param string|null $operations Some operations which need make on response |
||
108 | * @param string|null $tag Mark query with tag |
||
109 | * @return \RouterOS\Client |
||
110 | * @throws \RouterOS\Exceptions\QueryException |
||
111 | * @throws \RouterOS\Exceptions\ClientException |
||
112 | * @since 1.0.0 |
||
113 | */ |
||
114 | 4 | public function query(string $endpoint, array $where = null, string $operations = null, string $tag = null): Client |
|
115 | { |
||
116 | // If endpoint is string then build Query object |
||
117 | 4 | $query = new Query($endpoint); |
|
118 | |||
119 | // Parse where array |
||
120 | 4 | if (!empty($where)) { |
|
121 | |||
122 | // If array is multidimensional, then parse each line |
||
123 | 3 | if (is_array($where[0])) { |
|
124 | 1 | foreach ($where as $item) { |
|
125 | |||
126 | // Null by default |
||
127 | 1 | $key = null; |
|
128 | 1 | $operator = null; |
|
129 | 1 | $value = null; |
|
130 | |||
131 | 1 | View Code Duplication | switch (\count($item)) { |
|
|||
132 | 1 | case 1: |
|
133 | list($key) = $item; |
||
134 | break; |
||
135 | 1 | case 2: |
|
136 | list($key, $operator) = $item; |
||
137 | break; |
||
138 | 1 | case 3: |
|
139 | list($key, $operator, $value) = $item; |
||
140 | break; |
||
141 | default: |
||
142 | 1 | throw new ClientException('From 1 to 3 parameters of "where" condition is allowed'); |
|
143 | } |
||
144 | $query->where($key, $operator, $value); |
||
145 | } |
||
146 | } else { |
||
147 | // Null by default |
||
148 | 2 | $key = null; |
|
149 | 2 | $operator = null; |
|
150 | 2 | $value = null; |
|
151 | |||
152 | 2 | View Code Duplication | switch (\count($where)) { |
153 | 2 | case 1: |
|
154 | 1 | list($key) = $where; |
|
155 | 1 | break; |
|
156 | 2 | case 2: |
|
157 | 1 | list($key, $operator) = $where; |
|
158 | 1 | break; |
|
159 | 1 | case 3: |
|
160 | list($key, $operator, $value) = $where; |
||
161 | break; |
||
162 | default: |
||
163 | 1 | throw new ClientException('From 1 to 3 parameters of "where" condition is allowed'); |
|
164 | } |
||
165 | |||
166 | 1 | $query->where($key, $operator, $value); |
|
167 | } |
||
168 | |||
169 | } |
||
170 | |||
171 | // Append operations if set |
||
172 | 2 | if (!empty($operations)) { |
|
173 | $query->operations($operations); |
||
174 | } |
||
175 | |||
176 | // Append tag if set |
||
177 | 2 | if (!empty($tag)) { |
|
178 | $query->tag($tag); |
||
179 | } |
||
180 | |||
181 | // Submit query to RouterOS |
||
182 | 2 | return $this->writeRAW($query); |
|
183 | } |
||
184 | |||
185 | /** |
||
186 | * Send write query object to RouterOS |
||
187 | * |
||
188 | * @param \RouterOS\Query $query |
||
189 | * @return \RouterOS\Client |
||
190 | * @throws \RouterOS\Exceptions\QueryException |
||
191 | * @since 1.0.0 |
||
192 | */ |
||
193 | 15 | private function writeRAW(Query $query): Client |
|
205 | |||
206 | /** |
||
207 | * Read RAW response from RouterOS |
||
208 | * |
||
209 | * @return array |
||
210 | * @since 1.0.0 |
||
211 | */ |
||
212 | 15 | private function readRAW(): array |
|
248 | |||
249 | /** |
||
250 | * Read answer from server after query was executed |
||
251 | * |
||
252 | * A Mikrotik reply is formed of blocks |
||
253 | * Each block starts with a word, one of ('!re', '!trap', '!done', '!fatal') |
||
254 | * Each block end with an zero byte (empty line) |
||
255 | * Reply ends with a complete !done or !fatal block (ended with 'empty line') |
||
256 | * A !fatal block precedes TCP connexion close |
||
257 | * |
||
258 | * @param bool $parse |
||
259 | * @return mixed |
||
260 | */ |
||
261 | 15 | public function read(bool $parse = true) |
|
269 | |||
270 | /** |
||
271 | * Read using Iterators to improve performance on large dataset |
||
272 | * |
||
273 | * @return \RouterOS\ResponseIterator |
||
274 | * @since 1.0.0 |
||
275 | */ |
||
276 | 4 | public function readAsIterator(): ResponseIterator |
|
280 | |||
281 | /** |
||
282 | * This method was created by memory save reasons, it convert response |
||
283 | * from RouterOS to readable array in safe way. |
||
284 | * |
||
285 | * @param array $raw Array RAW response from server |
||
286 | * @return mixed |
||
287 | * |
||
288 | * Based on RouterOSResponseArray solution by @arily |
||
289 | * |
||
290 | * @link https://github.com/arily/RouterOSResponseArray |
||
291 | * @since 1.0.0 |
||
292 | */ |
||
293 | 4 | private function rosario(array $raw): array |
|
324 | |||
325 | /** |
||
326 | * Parse response from Router OS |
||
327 | * |
||
328 | * @param array $response Response data |
||
329 | * @return array Array with parsed data |
||
330 | */ |
||
331 | 5 | public function parseResponse(array $response): array |
|
332 | { |
||
333 | 5 | $result = []; |
|
334 | 5 | $i = -1; |
|
335 | 5 | $lines = \count($response); |
|
336 | 5 | foreach ($response as $key => $value) { |
|
337 | switch ($value) { |
||
338 | 5 | case '!re': |
|
339 | 2 | $i++; |
|
340 | 2 | break; |
|
341 | 5 | case '!fatal': |
|
342 | 1 | $result = $response; |
|
343 | 1 | break 2; |
|
344 | 4 | case '!trap': |
|
345 | 4 | case '!done': |
|
346 | // Check for =ret=, .tag and any other following messages |
||
347 | 4 | for ($j = $key + 1; $j <= $lines; $j++) { |
|
348 | // If we have lines after current one |
||
349 | 4 | if (isset($response[$j])) { |
|
350 | 2 | $this->pregResponse($response[$j], $matches); |
|
351 | 2 | View Code Duplication | if (isset($matches[1][0], $matches[2][0])) { |
352 | 2 | $result['after'][$matches[1][0]] = $matches[2][0]; |
|
353 | } |
||
354 | } |
||
355 | } |
||
356 | 4 | break 2; |
|
357 | default: |
||
358 | 2 | $this->pregResponse($value, $matches); |
|
359 | 2 | View Code Duplication | if (isset($matches[1][0], $matches[2][0])) { |
360 | 2 | $result[$i][$matches[1][0]] = $matches[2][0]; |
|
361 | } |
||
362 | 2 | break; |
|
363 | } |
||
364 | } |
||
365 | 5 | return $result; |
|
366 | } |
||
367 | |||
368 | /** |
||
369 | * Parse result from RouterOS by regular expression |
||
370 | * |
||
371 | * @param string $value |
||
372 | * @param array $matches |
||
373 | */ |
||
374 | 4 | private function pregResponse(string $value, &$matches) |
|
378 | |||
379 | /** |
||
380 | * Authorization logic |
||
381 | * |
||
382 | * @param bool $legacyRetry Retry login if we detect legacy version of RouterOS |
||
383 | * @return bool |
||
384 | * @throws \RouterOS\Exceptions\ClientException |
||
385 | * @throws \RouterOS\Exceptions\ConfigException |
||
386 | * @throws \RouterOS\Exceptions\QueryException |
||
387 | */ |
||
388 | 15 | private function login(bool $legacyRetry = false): bool |
|
428 | |||
429 | /** |
||
430 | * Detect by login request if firmware is legacy |
||
431 | * |
||
432 | * @param array $response |
||
433 | * @return bool |
||
434 | * @throws ConfigException |
||
435 | */ |
||
436 | 14 | private function isLegacy(array &$response): bool |
|
440 | |||
441 | /** |
||
442 | * Connect to socket server |
||
443 | * |
||
444 | * @return bool |
||
445 | * @throws \RouterOS\Exceptions\ClientException |
||
446 | * @throws \RouterOS\Exceptions\ConfigException |
||
447 | * @throws \RouterOS\Exceptions\QueryException |
||
448 | */ |
||
449 | 16 | private function connect(): bool |
|
480 | } |
||
481 |
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.