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:
1 | <?php |
||
27 | class admin_currency_controller extends admin_main |
||
28 | { |
||
29 | protected $ppde_operator; |
||
30 | |||
31 | /** |
||
32 | * Constructor |
||
33 | * |
||
34 | * @param ContainerInterface $container Service container interface |
||
35 | * @param \phpbb\log\log $log The phpBB log system |
||
36 | * @param \skouat\ppde\operators\currency $ppde_operator_currency Operator object |
||
37 | * @param \phpbb\request\request $request Request object |
||
38 | * @param \phpbb\template\template $template Template object |
||
39 | * @param \phpbb\user $user User object |
||
40 | * |
||
41 | * @access public |
||
42 | */ |
||
43 | View Code Duplication | public function __construct(ContainerInterface $container, \phpbb\log\log $log, \skouat\ppde\operators\currency $ppde_operator_currency, \phpbb\request\request $request, \phpbb\template\template $template, \phpbb\user $user) |
|
1 ignored issue
–
show
|
|||
44 | { |
||
45 | $this->container = $container; |
||
46 | $this->log = $log; |
||
47 | $this->ppde_operator = $ppde_operator_currency; |
||
48 | $this->request = $request; |
||
49 | $this->template = $template; |
||
50 | $this->user = $user; |
||
51 | parent::__construct( |
||
52 | 'currency', |
||
53 | 'PPDE_DC', |
||
54 | 'currency' |
||
55 | ); |
||
56 | } |
||
57 | |||
58 | /** |
||
59 | * Display the currency list |
||
60 | * |
||
61 | * @return null |
||
62 | * @access public |
||
63 | */ |
||
64 | public function display_currency() |
||
97 | |||
98 | /** |
||
99 | * Add a currency |
||
100 | * |
||
101 | * @return null |
||
102 | * @access public |
||
103 | */ |
||
104 | public function add_currency() |
||
132 | |||
133 | /** |
||
134 | * Process currency data to be added or edited |
||
135 | * |
||
136 | * @param \skouat\ppde\entity\currency $entity The currency entity object |
||
137 | * @param array $data The form data to be processed |
||
138 | * |
||
139 | * @return null |
||
140 | * @access private |
||
141 | */ |
||
142 | private function add_edit_currency_data($entity, $data) |
||
184 | |||
185 | /** |
||
186 | * Submit data to the database |
||
187 | * |
||
188 | * @param \skouat\ppde\entity\currency $entity The currency entity object |
||
189 | * @param array $errors |
||
190 | * |
||
191 | * @return null |
||
192 | * @access private |
||
193 | */ |
||
194 | private function submit_data($entity, array $errors) |
||
209 | |||
210 | /** |
||
211 | * Edit a Currency |
||
212 | * |
||
213 | * @param int $currency_id Currency Identifier |
||
214 | * |
||
215 | * @return null |
||
216 | * @access public |
||
217 | */ |
||
218 | public function edit_currency($currency_id) |
||
219 | { |
||
220 | // Add form key |
||
221 | add_form_key('add_edit_currency'); |
||
222 | |||
223 | // Initiate an entity |
||
224 | /** @type \skouat\ppde\entity\currency $entity */ |
||
225 | $entity = $this->get_container_entity(); |
||
226 | $entity->set_page_url($this->u_action); |
||
227 | $entity->load($currency_id); |
||
228 | |||
229 | // Collect the form data |
||
230 | $data = array( |
||
231 | 'currency_id' => $entity->get_id(), |
||
232 | 'currency_name' => $this->request->variable('currency_name', $entity->get_name(), true), |
||
233 | 'currency_iso_code' => $this->request->variable('currency_iso_code', $entity->get_iso_code(), true), |
||
234 | 'currency_symbol' => $this->request->variable('currency_symbol', $entity->get_symbol(), true), |
||
235 | 'currency_on_left' => $this->request->variable('currency_on_left', $entity->get_currency_position()), |
||
236 | 'currency_enable' => $this->request->variable('currency_enable', $entity->get_currency_enable()), |
||
237 | ); |
||
238 | |||
239 | // Process the new page |
||
240 | $this->add_edit_currency_data($entity, $data); |
||
241 | |||
242 | // Set output vars for display in the template |
||
243 | $this->template->assign_vars(array( |
||
244 | 'S_EDIT' => true, |
||
245 | 'U_EDIT_ACTION' => $this->u_action . '&action=edit&' . $this->id_prefix_name . '_id=' . $currency_id, |
||
246 | 'U_BACK' => $this->u_action, |
||
247 | )); |
||
248 | } |
||
249 | |||
250 | /** |
||
251 | * Move a currency up/down |
||
252 | * |
||
253 | * @param int $currency_id The currency identifier to move |
||
254 | * @param string $direction The direction (up|down) |
||
255 | * |
||
256 | * @return null |
||
257 | * @access public |
||
258 | */ |
||
259 | public function move_currency($currency_id, $direction) |
||
292 | |||
293 | /** |
||
294 | * Enable/disable a currency |
||
295 | * |
||
296 | * @param int $currency_id |
||
297 | * @param string $action |
||
298 | * |
||
299 | * @return null |
||
300 | * @access public |
||
301 | */ |
||
302 | public function enable_currency($currency_id, $action) |
||
337 | |||
338 | /** |
||
339 | * Delete a currency |
||
340 | * |
||
341 | * @param int $currency_id |
||
342 | * |
||
343 | * @return null |
||
344 | * @access public |
||
345 | */ |
||
346 | public function delete_currency($currency_id) |
||
358 | } |
||
359 |
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.