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 u2f 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 u2f, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
26 | class u2f extends abstract_module |
||
27 | { |
||
28 | /** |
||
29 | * @var request_interface |
||
30 | */ |
||
31 | private $request; |
||
32 | |||
33 | /** |
||
34 | * @var string |
||
35 | */ |
||
36 | private $registration_table; |
||
37 | |||
38 | /** |
||
39 | * @var string |
||
40 | */ |
||
41 | private $root_path; |
||
42 | |||
43 | /** |
||
44 | * @var \paul999\u2f\U2F |
||
45 | */ |
||
46 | private $u2f; |
||
47 | |||
48 | /** |
||
49 | * u2f constructor. |
||
50 | * @param driver_interface $db |
||
51 | * @param user $user |
||
52 | * @param request_interface $request |
||
53 | * @param template $template |
||
54 | * @param string $registration_table |
||
55 | * @param string $root_path |
||
56 | */ |
||
57 | public function __construct(driver_interface $db, user $user, request_interface $request, template $template, $registration_table, $root_path) |
||
69 | |||
70 | /** |
||
71 | * Return if this module is enabled by the admin |
||
72 | * (And all server requirements are met). |
||
73 | * |
||
74 | * Do not return false in case a specific user disabeld this module, |
||
75 | * OR if the user is unable to use this specific module. |
||
76 | * @return boolean |
||
77 | */ |
||
78 | public function is_enabled() |
||
82 | |||
83 | /** |
||
84 | * Check if the current user is able to use this module. |
||
85 | * |
||
86 | * This means that the user enabled it in the UCP, |
||
87 | * And has it setup up correctly. |
||
88 | * This method will be called during login, not during registration/ |
||
89 | * |
||
90 | * @param int $user_id |
||
91 | * @return bool |
||
92 | */ |
||
93 | public function is_usable($user_id) |
||
101 | |||
102 | /** |
||
103 | * Check if the user can potentially use this. |
||
104 | * This method is called at registration page. |
||
105 | * |
||
106 | * You can, for example, check if the current browser is suitable. |
||
107 | * |
||
108 | * @param int|boolean $user_id Use false to ignore user |
||
109 | * @return bool |
||
110 | */ |
||
111 | public function is_potentially_usable($user_id = false) |
||
117 | |||
118 | /** |
||
119 | * Check if the current session is secure. |
||
120 | * |
||
121 | * @return bool |
||
122 | */ |
||
123 | private function is_ssl() |
||
136 | |||
137 | /** |
||
138 | * Get the priority for this module. |
||
139 | * A lower priority means more chance it gets selected as default option |
||
140 | * |
||
141 | * There can be only one module with a specific priority! |
||
142 | * If there is already a module registered with this priority, |
||
143 | * a Exception might be thrown |
||
144 | * |
||
145 | * @return int |
||
146 | */ |
||
147 | public function get_priority() |
||
151 | |||
152 | /** |
||
153 | * Start of the login procedure. |
||
154 | * @param int $user_id |
||
155 | * @return array |
||
156 | * @throws http_exception |
||
157 | */ |
||
158 | public function login_start($user_id) |
||
181 | |||
182 | /** |
||
183 | * Actual login procedure |
||
184 | * |
||
185 | * @param int $user_id |
||
186 | * |
||
187 | * @return bool |
||
188 | * @throws http_exception |
||
189 | */ |
||
190 | public function login($user_id) |
||
242 | |||
243 | /** |
||
244 | * @param array $requests |
||
245 | * @return array |
||
246 | */ |
||
247 | private function convertRequests($requests) |
||
256 | |||
257 | /** |
||
258 | * Start of registration |
||
259 | * @return string |
||
260 | */ |
||
261 | public function register_start() |
||
290 | |||
291 | /** |
||
292 | * Actual registration |
||
293 | * @throws http_exception |
||
294 | */ |
||
295 | public function register() |
||
337 | |||
338 | /** |
||
339 | * This method is called to show the UCP page. |
||
340 | * You can assign template variables to the template, or do anything else here. |
||
341 | */ |
||
342 | public function show_ucp() |
||
346 | |||
347 | /** |
||
348 | * Delete a specific row from the UCP. |
||
349 | * The data is based on the data provided in show_ucp. |
||
350 | * @param int $key |
||
351 | * @return void |
||
352 | */ |
||
353 | View Code Duplication | public function delete($key) |
|
361 | |||
362 | /** |
||
363 | * If this module can add new keys (Or other things) |
||
364 | * |
||
365 | * @return boolean |
||
366 | */ |
||
367 | public function can_register() |
||
371 | |||
372 | /** |
||
373 | * Return the name of the current module |
||
374 | * This is for internal use only |
||
375 | * @return string |
||
376 | */ |
||
377 | public function get_name() |
||
381 | |||
382 | /** |
||
383 | * Get a language key for this specific module. |
||
384 | * @return string |
||
385 | */ |
||
386 | public function get_translatable_name() |
||
390 | |||
391 | /** |
||
392 | * Select all registration objects from the database |
||
393 | * @param integer $user_id |
||
394 | * @return array |
||
395 | */ |
||
396 | private function getRegistrations($user_id) |
||
417 | |||
418 | /** |
||
419 | * @param U2fError $error |
||
420 | * @throws http_exception |
||
421 | */ |
||
422 | private function createError(U2fError $error) |
||
478 | |||
479 | /** |
||
480 | * Update the session with new TFA data |
||
481 | * @param $sql_ary |
||
482 | * @return int |
||
483 | */ |
||
484 | private function update_session($sql_ary) |
||
494 | } |
||
495 |
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.