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 CurlFtpAdapter 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 CurlFtpAdapter, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class CurlFtpAdapter extends AbstractFtpAdapter |
||
15 | { |
||
16 | protected $configurable = [ |
||
17 | 'protocol', |
||
18 | 'host', |
||
19 | 'port', |
||
20 | 'username', |
||
21 | 'password', |
||
22 | 'root', |
||
23 | ]; |
||
24 | |||
25 | /** @var Curl */ |
||
26 | protected $connection; |
||
27 | |||
28 | /** @var bool */ |
||
29 | protected $isPureFtpd; |
||
30 | |||
31 | /** |
||
32 | * Establish a connection. |
||
33 | */ |
||
34 | public function connect() |
||
50 | |||
51 | /** |
||
52 | * Close the connection. |
||
53 | */ |
||
54 | public function disconnect() |
||
61 | |||
62 | /** |
||
63 | * Check if a connection is active. |
||
64 | * |
||
65 | * @return bool |
||
66 | */ |
||
67 | public function isConnected() |
||
71 | |||
72 | /** |
||
73 | * Write a new file. |
||
74 | * |
||
75 | * @param string $path |
||
76 | * @param string $contents |
||
77 | * @param Config $config Config object |
||
78 | * |
||
79 | * @return array|false false on failure file meta data on success |
||
80 | */ |
||
81 | public function write($path, $contents, Config $config) |
||
98 | |||
99 | /** |
||
100 | * Write a new file using a stream. |
||
101 | * |
||
102 | * @param string $path |
||
103 | * @param resource $resource |
||
104 | * @param Config $config Config object |
||
105 | * |
||
106 | * @return array|false false on failure file meta data on success |
||
107 | */ |
||
108 | public function writeStream($path, $resource, Config $config) |
||
126 | |||
127 | /** |
||
128 | * Update a file. |
||
129 | * |
||
130 | * @param string $path |
||
131 | * @param string $contents |
||
132 | * @param Config $config Config object |
||
133 | * |
||
134 | * @return array|false false on failure file meta data on success |
||
135 | */ |
||
136 | public function update($path, $contents, Config $config) |
||
140 | |||
141 | /** |
||
142 | * Update a file using a stream. |
||
143 | * |
||
144 | * @param string $path |
||
145 | * @param resource $resource |
||
146 | * @param Config $config Config object |
||
147 | * |
||
148 | * @return array|false false on failure file meta data on success |
||
149 | */ |
||
150 | public function updateStream($path, $resource, Config $config) |
||
154 | |||
155 | /** |
||
156 | * Rename a file. |
||
157 | * |
||
158 | * @param string $path |
||
159 | * @param string $newpath |
||
160 | * |
||
161 | * @return bool |
||
162 | */ |
||
163 | public function rename($path, $newpath) |
||
181 | |||
182 | /** |
||
183 | * Copy a file. |
||
184 | * |
||
185 | * @param string $path |
||
186 | * @param string $newpath |
||
187 | * |
||
188 | * @return bool |
||
189 | */ |
||
190 | public function copy($path, $newpath) |
||
200 | |||
201 | /** |
||
202 | * Delete a file. |
||
203 | * |
||
204 | * @param string $path |
||
205 | * |
||
206 | * @return bool |
||
207 | */ |
||
208 | View Code Duplication | public function delete($path) |
|
220 | |||
221 | /** |
||
222 | * Delete a directory. |
||
223 | * |
||
224 | * @param string $dirname |
||
225 | * |
||
226 | * @return bool |
||
227 | */ |
||
228 | View Code Duplication | public function deleteDir($dirname) |
|
240 | |||
241 | /** |
||
242 | * Create a directory. |
||
243 | * |
||
244 | * @param string $dirname directory name |
||
245 | * @param Config $config |
||
246 | * |
||
247 | * @return array|false |
||
248 | */ |
||
249 | public function createDir($dirname, Config $config) |
||
261 | |||
262 | /** |
||
263 | * Set the visibility for a file. |
||
264 | * |
||
265 | * @param string $path |
||
266 | * @param string $visibility |
||
267 | * |
||
268 | * @return array|false file meta data |
||
269 | */ |
||
270 | public function setVisibility($path, $visibility) |
||
289 | |||
290 | /** |
||
291 | * Read a file. |
||
292 | * |
||
293 | * @param string $path |
||
294 | * |
||
295 | * @return array|false |
||
296 | */ |
||
297 | public function read($path) |
||
309 | |||
310 | /** |
||
311 | * Read a file as a stream. |
||
312 | * |
||
313 | * @param string $path |
||
314 | * |
||
315 | * @return array|false |
||
316 | */ |
||
317 | public function readStream($path) |
||
338 | |||
339 | /** |
||
340 | * Get all the meta data of a file or directory. |
||
341 | * |
||
342 | * @param string $path |
||
343 | * |
||
344 | * @return array|false |
||
345 | */ |
||
346 | public function getMetadata($path) |
||
363 | |||
364 | /** |
||
365 | * Get the mimetype of a file. |
||
366 | * |
||
367 | * @param string $path |
||
368 | * |
||
369 | * @return array|false |
||
370 | */ |
||
371 | public function getMimetype($path) |
||
381 | |||
382 | /** |
||
383 | * Get the timestamp of a file. |
||
384 | * |
||
385 | * @param string $path |
||
386 | * |
||
387 | * @return array|false |
||
388 | */ |
||
389 | public function getTimestamp($path) |
||
401 | |||
402 | /** |
||
403 | * {@inheritdoc} |
||
404 | * |
||
405 | * @param string $directory |
||
406 | */ |
||
407 | protected function listDirectoryContents($directory, $recursive = false) |
||
427 | |||
428 | /** |
||
429 | * {@inheritdoc} |
||
430 | * |
||
431 | * @param string $directory |
||
432 | */ |
||
433 | protected function listDirectoryContentsRecursive($directory) |
||
454 | |||
455 | /** |
||
456 | * Normalize a permissions string. |
||
457 | * |
||
458 | * @param string $permissions |
||
459 | * |
||
460 | * @return int |
||
461 | */ |
||
462 | protected function normalizePermissions($permissions) |
||
479 | |||
480 | /** |
||
481 | * Normalize path depending on server. |
||
482 | * |
||
483 | * @param string $path |
||
484 | * |
||
485 | * @return string |
||
486 | */ |
||
487 | protected function normalizePath($path) |
||
502 | |||
503 | /** |
||
504 | * @return bool |
||
505 | */ |
||
506 | protected function isPureFtpdServer() |
||
516 | |||
517 | /** |
||
518 | * Sends an arbitrary command to an FTP server. |
||
519 | * |
||
520 | * @param Curl $connection The CURL instance |
||
521 | * @param string $command The command to execute |
||
522 | * |
||
523 | * @return array Returns the server's response as an array of strings |
||
524 | */ |
||
525 | protected function rawCommand($connection, $command) |
||
526 | { |
||
527 | $response = ''; |
||
528 | $callback = function ($ch, $string) use (&$response) { |
||
529 | $response .= $string; |
||
530 | |||
531 | return strlen($string); |
||
532 | }; |
||
533 | $connection->exec([ |
||
534 | CURLOPT_CUSTOMREQUEST => $command, |
||
535 | CURLOPT_HEADERFUNCTION => $callback, |
||
536 | ]); |
||
537 | |||
538 | return explode(PHP_EOL, trim($response)); |
||
539 | } |
||
540 | |||
541 | /** |
||
542 | * Returns the base url of the connection. |
||
543 | * |
||
544 | * @return string |
||
545 | */ |
||
546 | protected function getBaseUri() |
||
552 | |||
553 | /** |
||
554 | * Check the connection is established. |
||
555 | */ |
||
556 | protected function pingConnection() |
||
563 | |||
564 | /** |
||
565 | * Set the connection root. |
||
566 | */ |
||
567 | protected function setConnectionRoot() |
||
581 | } |
||
582 |
This checks looks for assignemnts to variables using the
list(...)
function, where not all assigned variables are subsequently used.Consider the following code example.
Only the variables
$a
and$c
are used. There was no need to assign$b
.Instead, the list call could have been.