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 |
||
21 | class Client |
||
22 | { |
||
23 | /** |
||
24 | * Onesky API endpoint |
||
25 | */ |
||
26 | protected $endpoint = 'https://platform.api.onesky.io/1'; |
||
27 | |||
28 | /** |
||
29 | * Client authenticate token |
||
30 | */ |
||
31 | protected $apiKey = null; |
||
32 | |||
33 | /** |
||
34 | * Client authenticate secret |
||
35 | */ |
||
36 | protected $secret = null; |
||
37 | |||
38 | /** |
||
39 | * Resources with actions |
||
40 | */ |
||
41 | protected $resources = array( |
||
42 | 'project_groups' => array( |
||
43 | 'list' => '/project-groups', |
||
44 | 'show' => '/project-groups/:project_group_id', |
||
45 | 'create' => '/project-groups', |
||
46 | 'delete' => '/project-groups/:project_group_id', |
||
47 | 'languages' => '/project-groups/:project_group_id/languages', |
||
48 | ), |
||
49 | 'projects' => array( |
||
50 | 'list' => '/project-groups/:project_group_id/projects', |
||
51 | 'show' => '/projects/:project_id', |
||
52 | 'create' => '/project-groups/:project_group_id/projects', |
||
53 | 'update' => '/projects/:project_id', |
||
54 | 'delete' => '/projects/:project_id', |
||
55 | 'languages' => '/projects/:project_id/languages', |
||
56 | ), |
||
57 | 'files' => array( |
||
58 | 'list' => '/projects/:project_id/files', |
||
59 | 'upload' => '/projects/:project_id/files', |
||
60 | 'delete' => '/projects/:project_id/files', |
||
61 | ), |
||
62 | 'translations' => array( |
||
63 | 'export' => '/projects/:project_id/translations', |
||
64 | 'multilingual' => '/projects/:project_id/translations/multilingual', |
||
65 | 'status' => '/projects/:project_id/translations/status', |
||
66 | ), |
||
67 | 'import_tasks' => array( |
||
68 | 'list' => '/projects/:project_id/import-tasks/', |
||
69 | 'show' => '/projects/:project_id/import-tasks/:import_id' |
||
70 | ), |
||
71 | 'quotations' => array( |
||
72 | 'show' => '/projects/:project_id/quotations' |
||
73 | ), |
||
74 | 'orders' => array( |
||
75 | 'list' => '/projects/:project_id/orders', |
||
76 | 'show' => '/projects/:project_id/orders/:order_id', |
||
77 | 'create' => '/projects/:project_id/orders' |
||
78 | ), |
||
79 | 'locales' => array( |
||
80 | 'list' => '/locales' |
||
81 | ), |
||
82 | 'project_types' => array( |
||
83 | 'list' => '/project-types' |
||
84 | ), |
||
85 | ); |
||
86 | |||
87 | /** |
||
88 | * Actions to use multipart to upload file |
||
89 | */ |
||
90 | protected $multiPartActions = array( |
||
91 | 'files' => array('upload'), |
||
92 | ); |
||
93 | |||
94 | /** |
||
95 | * Actions to use multipart to upload file |
||
96 | */ |
||
97 | protected $exportFileActions = array( |
||
98 | 'translations' => array('export'), |
||
99 | ); |
||
100 | |||
101 | /** |
||
102 | * Methods of actions mapping |
||
103 | */ |
||
104 | protected $methods = array( |
||
105 | // 'get' => array('list', 'show', 'languages', 'export', 'status'), |
||
|
|||
106 | 'post' => array('create', 'upload'), |
||
107 | 'put' => array('update'), |
||
108 | 'delete' => array('delete'), |
||
109 | ); |
||
110 | |||
111 | /** |
||
112 | * Default curl settings |
||
113 | */ |
||
114 | protected $curlSettings = array( |
||
115 | CURLOPT_RETURNTRANSFER => true, |
||
116 | ); |
||
117 | |||
118 | protected $httpHeaders = array( |
||
119 | "Onesky-Plugin: php-wrapper", |
||
120 | ); |
||
121 | |||
122 | 26 | public function __construct() |
|
128 | |||
129 | /** |
||
130 | * @param string $apiKey |
||
131 | * @return $this |
||
132 | */ |
||
133 | public function setApiKey($apiKey) |
||
138 | |||
139 | /** |
||
140 | * @param string $secret |
||
141 | * @return $this |
||
142 | */ |
||
143 | public function setSecret($secret) |
||
148 | |||
149 | /** |
||
150 | * Retrieve resources |
||
151 | * @return array |
||
152 | */ |
||
153 | 25 | public function getResources() |
|
157 | |||
158 | /** |
||
159 | * Retrieve actions of a resource |
||
160 | * @param string $resource Resource name |
||
161 | * @return array|null |
||
162 | */ |
||
163 | 25 | public function getActionsByResource($resource) |
|
176 | |||
177 | /** |
||
178 | * Retrieve the corresponding method to use |
||
179 | * @param string $action Action name |
||
180 | * @return string |
||
181 | */ |
||
182 | 22 | public function getMethodByAction($action) |
|
192 | |||
193 | /** |
||
194 | * Determine if using mulit-part to upload file |
||
195 | * @param string $resource Resource name |
||
196 | * @param string $action Action name |
||
197 | * @return boolean |
||
198 | */ |
||
199 | 11 | public function isMultiPartAction($resource, $action) |
|
203 | |||
204 | /** |
||
205 | * Determine if it is to export (download) file |
||
206 | * @param string $resource Resource name |
||
207 | * @param string $action Action name |
||
208 | * @return boolean |
||
209 | */ |
||
210 | public function isExportFileAction($resource, $action) |
||
214 | |||
215 | /** |
||
216 | * For developers to initial request to Onesky API |
||
217 | * |
||
218 | * Example: |
||
219 | * $onesky = new Onesky_Api(); |
||
220 | * $onesky->setApiKey('<api-key>')->setSecret('<api-secret>'); |
||
221 | * |
||
222 | * // To list project type |
||
223 | * $onesky->projectTypes('list'); |
||
224 | * |
||
225 | * // To create project |
||
226 | * $onesky->projects('create', array('project_group_id' => 999)); |
||
227 | * |
||
228 | * // To upload string file |
||
229 | * $onesky->files('upload', array('project_id' => 1099, 'file' => 'path/to/file.yml', 'file_format' => 'YAML')); |
||
230 | * |
||
231 | * @param string $fn_name Function name acted as resource name |
||
232 | * @param array $params Parameters passed in request |
||
233 | * @return array Response |
||
234 | */ |
||
235 | 25 | public function __call($fn_name, $params) |
|
268 | |||
269 | /** |
||
270 | * Retrieve request path and replace variables with values |
||
271 | * @param string $resource Resource name |
||
272 | * @param string $action Action name |
||
273 | * @param array $params Parameters |
||
274 | * @return string Request path |
||
275 | */ |
||
276 | 22 | private function getRequestPath($resource, $action, &$params) |
|
300 | |||
301 | 11 | protected function verifyTokenAndSecret() |
|
307 | |||
308 | /** |
||
309 | * Initial request to Onesky API |
||
310 | * @param string $method |
||
311 | * @param string $path |
||
312 | * @param array $params |
||
313 | * @param boolean $isMultiPart |
||
314 | * @return array |
||
315 | */ |
||
316 | 11 | private function callApi($method, $path, $params, $isMultiPart) |
|
385 | |||
386 | /** |
||
387 | * @param $params |
||
388 | * @return string |
||
389 | */ |
||
390 | 6 | private function getAuthQueryStringWithParams($params) |
|
400 | |||
401 | /** |
||
402 | * @return string |
||
403 | */ |
||
404 | 11 | private function getAuthQueryString() |
|
417 | |||
418 | /** |
||
419 | * @param array $params |
||
420 | * @return array |
||
421 | */ |
||
422 | 11 | private function normalizeParams(array $params) |
|
433 | } |
||
434 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.