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 |
||
| 16 | class admin_controller |
||
| 17 | { |
||
| 18 | /** @var \phpbb\db\driver\driver_interface */ |
||
| 19 | protected $db; |
||
| 20 | |||
| 21 | /** @var \phpbb\template\template */ |
||
| 22 | protected $template; |
||
| 23 | |||
| 24 | /** @var \phpbb\user */ |
||
| 25 | protected $user; |
||
| 26 | |||
| 27 | /** @var \phpbb\request\request */ |
||
| 28 | protected $request; |
||
| 29 | |||
| 30 | /** @var string ads_table */ |
||
| 31 | protected $ads_table; |
||
| 32 | |||
| 33 | /** @var string Custom form action */ |
||
| 34 | protected $u_action; |
||
| 35 | |||
| 36 | /** |
||
| 37 | * Constructor |
||
| 38 | * |
||
| 39 | * @param \phpbb\db\driver\driver_interface $db DB driver interface |
||
| 40 | * @param \phpbb\template\template $template Template object |
||
| 41 | * @param \phpbb\user $user User object |
||
| 42 | * @param \phpbb\request\request $request Request object |
||
| 43 | * @param string $ads_table Ads table |
||
| 44 | */ |
||
| 45 | public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\template\template $template, \phpbb\user $user, \phpbb\request\request $request, $ads_table) |
||
| 53 | |||
| 54 | /** |
||
| 55 | * Set page url |
||
| 56 | * |
||
| 57 | * @param string $u_action Custom form action |
||
| 58 | * @return void |
||
| 59 | */ |
||
| 60 | public function set_page_url($u_action) |
||
| 64 | |||
| 65 | /** |
||
| 66 | * Load module-specific language |
||
| 67 | * |
||
| 68 | * @return void |
||
| 69 | */ |
||
| 70 | public function load_lang() |
||
| 74 | |||
| 75 | /** |
||
| 76 | * Get ACP page title for Ads module |
||
| 77 | * |
||
| 78 | * @return string Language string for Ads ACP module |
||
| 79 | */ |
||
| 80 | public function get_page_title() |
||
| 84 | |||
| 85 | /** |
||
| 86 | * Get action |
||
| 87 | * |
||
| 88 | * @return string Ads module action |
||
| 89 | */ |
||
| 90 | public function get_action() |
||
| 94 | |||
| 95 | /** |
||
| 96 | * Add an advertisement |
||
| 97 | * |
||
| 98 | * @return void |
||
| 99 | */ |
||
| 100 | public function action_add() |
||
| 157 | |||
| 158 | /** |
||
| 159 | * Edit an advertisement |
||
| 160 | * |
||
| 161 | * @return void |
||
| 162 | */ |
||
| 163 | public function action_edit() |
||
| 238 | |||
| 239 | /** |
||
| 240 | * Enable/disable an advertisement |
||
| 241 | * |
||
| 242 | * @param bool $enable Enable or disable the advertisement? |
||
| 243 | * @return void |
||
| 244 | */ |
||
| 245 | public function ad_enable($enable) |
||
| 273 | |||
| 274 | /** |
||
| 275 | * Delete an advertisement |
||
| 276 | * |
||
| 277 | * @return void |
||
| 278 | */ |
||
| 279 | public function action_delete() |
||
| 280 | { |
||
| 281 | $ad_id = $this->request->variable('id', 0); |
||
| 282 | |||
| 283 | $sql = 'SELECT ad_id |
||
| 284 | FROM ' . $this->ads_table . ' |
||
| 285 | WHERE ad_id = ' . (int) $ad_id; |
||
| 286 | $result = $this->db->sql_query($sql); |
||
| 287 | $row = $this->db->sql_fetchrow($result); |
||
| 288 | $this->db->sql_freeresult($result); |
||
| 289 | |||
| 290 | if ($row) |
||
| 291 | { |
||
| 292 | if (confirm_box(true)) |
||
| 293 | { |
||
| 294 | $sql = 'DELETE FROM ' . $this->ads_table . ' |
||
| 295 | WHERE ad_id = ' . (int) $ad_id; |
||
| 296 | $this->db->sql_query($sql); |
||
| 297 | |||
| 298 | // Only notify user on error |
||
| 299 | if (!$this->db->sql_affectedrows()) |
||
| 300 | { |
||
| 301 | trigger_error($this->user->lang('ACP_AD_DELETE_ERRORED') . adm_back_link($this->u_action), E_USER_WARNING); |
||
| 302 | } |
||
| 303 | } |
||
| 304 | else |
||
| 305 | { |
||
| 306 | confirm_box(false, $this->user->lang('CONFIRM_OPERATION'), build_hidden_fields(array( |
||
| 307 | 'id' => $ad_id, |
||
| 308 | 'i' => $this->request->variable('i', ''), |
||
| 309 | 'mode' => $this->request->variable('mode', ''), |
||
| 310 | 'action' => 'delete')) |
||
| 311 | ); |
||
| 312 | } |
||
| 313 | } |
||
| 314 | } |
||
| 315 | |||
| 316 | |||
| 317 | /** |
||
| 318 | * Display the ads |
||
| 319 | * |
||
| 320 | * @return void |
||
| 321 | */ |
||
| 322 | public function list_ads() |
||
| 347 | } |
||
| 348 |
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.