phpbb-extensions /
ad-management
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 | const MAX_NAME_LENGTH = 255; |
||
| 19 | |||
| 20 | /** @var \phpbb\db\driver\driver_interface */ |
||
| 21 | protected $db; |
||
| 22 | |||
| 23 | /** @var \phpbb\template\template */ |
||
| 24 | protected $template; |
||
| 25 | |||
| 26 | /** @var \phpbb\user */ |
||
| 27 | protected $user; |
||
| 28 | |||
| 29 | /** @var \phpbb\request\request */ |
||
| 30 | protected $request; |
||
| 31 | |||
| 32 | /** @var string ads_table */ |
||
| 33 | protected $ads_table; |
||
| 34 | |||
| 35 | /** @var string php_ext */ |
||
| 36 | protected $php_ext; |
||
| 37 | |||
| 38 | /** @var string phpbb_admin_path */ |
||
| 39 | protected $phpbb_admin_path; |
||
| 40 | |||
| 41 | /** @var string Custom form action */ |
||
| 42 | protected $u_action; |
||
| 43 | |||
| 44 | /** @var array Form validation errors */ |
||
| 45 | protected $errors = array(); |
||
| 46 | |||
| 47 | /** |
||
| 48 | * Constructor |
||
| 49 | * |
||
| 50 | * @param \phpbb\db\driver\driver_interface $db DB driver interface |
||
| 51 | * @param \phpbb\template\template $template Template object |
||
| 52 | * @param \phpbb\user $user User object |
||
| 53 | * @param \phpbb\request\request $request Request object |
||
| 54 | * @param string $ads_table Ads table |
||
| 55 | * @param string $php_ext PHP extension |
||
| 56 | * @param string $phpbb_admin_path Path to admin |
||
| 57 | */ |
||
| 58 | 5 | public function __construct(\phpbb\db\driver\driver_interface $db, \phpbb\template\template $template, \phpbb\user $user, \phpbb\request\request $request, $ads_table, $php_ext, $phpbb_admin_path) |
|
| 68 | |||
| 69 | /** |
||
| 70 | * Set page url |
||
| 71 | * |
||
| 72 | * @param string $u_action Custom form action |
||
| 73 | * @return void |
||
| 74 | */ |
||
| 75 | 5 | public function set_page_url($u_action) |
|
| 79 | |||
| 80 | /** |
||
| 81 | * Load module-specific language |
||
| 82 | * |
||
| 83 | * @return void |
||
| 84 | */ |
||
| 85 | 5 | public function load_lang() |
|
| 89 | |||
| 90 | /** |
||
| 91 | * Get ACP page title for Ads module |
||
| 92 | * |
||
| 93 | * @return string Language string for Ads ACP module |
||
| 94 | */ |
||
| 95 | 1 | public function get_page_title() |
|
| 99 | |||
| 100 | /** |
||
| 101 | * Get action |
||
| 102 | * |
||
| 103 | * @return string Ads module action |
||
| 104 | */ |
||
| 105 | 1 | public function get_action() |
|
| 109 | |||
| 110 | /** |
||
| 111 | * Add an advertisement |
||
| 112 | * |
||
| 113 | * @return void |
||
| 114 | */ |
||
| 115 | 2 | public function action_add() |
|
| 116 | { |
||
| 117 | 2 | add_form_key('phpbb/admanagement/add'); |
|
| 118 | 2 | if ($this->request->is_set_post('submit')) |
|
| 119 | 2 | { |
|
| 120 | 1 | $this->check_form_key('phpbb/admanagement/add'); |
|
| 121 | |||
| 122 | 1 | $data = $this->get_form_data(); |
|
| 123 | |||
| 124 | 1 | $this->validate($data); |
|
| 125 | |||
| 126 | 1 | View Code Duplication | if (empty($this->errors)) |
|
|
|||
| 127 | 1 | { |
|
| 128 | // Insert the ad data to the database |
||
| 129 | $sql = 'INSERT INTO ' . $this->ads_table . ' ' . $this->db->sql_build_array('INSERT', $data); |
||
| 130 | $this->db->sql_query($sql); |
||
| 131 | |||
| 132 | $this->success('ACP_AD_ADD_SUCCESS'); |
||
| 133 | } |
||
| 134 | else |
||
| 135 | { |
||
| 136 | 1 | $this->assign_errors(); |
|
| 137 | 1 | $this->assign_form_data($data); |
|
| 138 | } |
||
| 139 | 1 | } |
|
| 140 | |||
| 141 | // Set output vars for display in the template |
||
| 142 | 2 | $this->template->assign_vars(array( |
|
| 143 | 2 | 'S_ADD_AD' => true, |
|
| 144 | 2 | 'U_BACK' => $this->u_action, |
|
| 145 | 2 | )); |
|
| 146 | 2 | } |
|
| 147 | |||
| 148 | /** |
||
| 149 | * Edit an advertisement |
||
| 150 | * |
||
| 151 | * @return void |
||
| 152 | */ |
||
| 153 | public function action_edit() |
||
| 204 | |||
| 205 | /** |
||
| 206 | * Enable/disable an advertisement |
||
| 207 | * |
||
| 208 | * @param bool $enable Enable or disable the advertisement? |
||
| 209 | * @return void |
||
| 210 | */ |
||
| 211 | public function ad_enable($enable) |
||
| 239 | |||
| 240 | /** |
||
| 241 | * Delete an advertisement |
||
| 242 | * |
||
| 243 | * @return void |
||
| 244 | */ |
||
| 245 | public function action_delete() |
||
| 277 | |||
| 278 | |||
| 279 | /** |
||
| 280 | * Display the ads |
||
| 281 | * |
||
| 282 | * @return void |
||
| 283 | */ |
||
| 284 | 1 | public function list_ads() |
|
| 285 | { |
||
| 286 | $sql = 'SELECT ad_id, ad_name, ad_enabled |
||
| 287 | 1 | FROM ' . $this->ads_table; |
|
| 288 | 1 | $result = $this->db->sql_query($sql); |
|
| 289 | 1 | while ($row = $this->db->sql_fetchrow($result)) |
|
| 290 | { |
||
| 291 | 1 | $ad_enabled = (bool) $row['ad_enabled']; |
|
| 292 | |||
| 293 | 1 | $this->template->assign_block_vars('ads', array( |
|
| 294 | 1 | 'NAME' => $row['ad_name'], |
|
| 295 | 1 | 'S_ENABLED' => (int) $ad_enabled, |
|
| 296 | 1 | 'U_ENABLE' => $this->u_action . '&action=' . ($ad_enabled ? 'disable' : 'enable') . '&id=' . $row['ad_id'], |
|
| 297 | 1 | 'U_PREVIEW' => append_sid(generate_board_url() . '/index.' . $this->php_ext, 'ad_preview=' . $row['ad_id']), |
|
| 298 | 1 | 'U_EDIT' => $this->u_action . '&action=edit&id=' . $row['ad_id'], |
|
| 299 | 1 | 'U_DELETE' => $this->u_action . '&action=delete&id=' . $row['ad_id'], |
|
| 300 | 1 | )); |
|
| 301 | 1 | } |
|
| 302 | 1 | $this->db->sql_freeresult($result); |
|
| 303 | |||
| 304 | // Set output vars for display in the template |
||
| 305 | 1 | $this->template->assign_vars(array( |
|
| 306 | 1 | 'U_ACTION_ADD' => $this->u_action . '&action=add', |
|
| 307 | 1 | 'ICON_PREVIEW' => '<img src="' . htmlspecialchars($this->phpbb_admin_path) . 'images/file_up_to_date.gif" alt="' . $this->user->lang('AD_PREVIEW') . '" title="' . $this->user->lang('AD_PREVIEW') . '" />', |
|
| 308 | 1 | )); |
|
| 309 | 1 | } |
|
| 310 | |||
| 311 | /** |
||
| 312 | * Check the form key. |
||
| 313 | * |
||
| 314 | * @param string $form_name The name of the form. |
||
| 315 | * @return void |
||
| 316 | */ |
||
| 317 | 1 | protected function check_form_key($form_name) |
|
| 318 | { |
||
| 319 | 1 | if (!check_form_key($form_name)) |
|
| 320 | 1 | { |
|
| 321 | 1 | $this->errors[] = $this->user->lang('FORM_INVALID'); |
|
| 322 | 1 | } |
|
| 323 | 1 | } |
|
| 324 | |||
| 325 | /** |
||
| 326 | * Get admin form data. |
||
| 327 | * |
||
| 328 | * @return array Form data |
||
| 329 | */ |
||
| 330 | 1 | protected function get_form_data() |
|
| 331 | { |
||
| 332 | return array( |
||
| 333 | 1 | 'ad_name' => $this->request->variable('ad_name', '', true), |
|
| 334 | 1 | 'ad_note' => $this->request->variable('ad_note', '', true), |
|
| 335 | 1 | 'ad_code' => $this->request->variable('ad_code', '', true), |
|
| 336 | 1 | 'ad_enabled' => $this->request->variable('ad_enabled', false), |
|
| 337 | 1 | ); |
|
| 338 | } |
||
| 339 | |||
| 340 | /** |
||
| 341 | * Validate form data. |
||
| 342 | * |
||
| 343 | * @param array $data The form data. |
||
| 344 | * @return void |
||
| 345 | */ |
||
| 346 | 1 | protected function validate($data) |
|
| 347 | { |
||
| 348 | 1 | if ($data['ad_name'] === '') |
|
| 349 | 1 | { |
|
| 350 | $this->errors[] = $this->user->lang('AD_NAME_REQUIRED'); |
||
| 351 | } |
||
| 352 | 1 | if (truncate_string($data['ad_name'], self::MAX_NAME_LENGTH) !== $data['ad_name']) |
|
| 353 | 1 | { |
|
| 354 | $this->errors[] = $this->user->lang('AD_NAME_TOO_LONG', self::MAX_NAME_LENGTH); |
||
| 355 | } |
||
| 356 | 1 | } |
|
| 357 | |||
| 358 | /** |
||
| 359 | * Assign errors to the template. |
||
| 360 | * |
||
| 361 | * @return void |
||
| 362 | */ |
||
| 363 | 1 | protected function assign_errors() |
|
| 364 | { |
||
| 365 | 1 | $this->template->assign_vars(array( |
|
| 366 | 1 | 'S_ERROR' => (bool) count($this->errors), |
|
| 367 | 1 | 'ERROR_MSG' => count($this->errors) ? implode('<br />', $this->errors) : '', |
|
| 368 | 1 | )); |
|
| 369 | 1 | } |
|
| 370 | |||
| 371 | /** |
||
| 372 | * Assign form data to the template. |
||
| 373 | * |
||
| 374 | * @param array $data The form data. |
||
| 375 | * @return void |
||
| 376 | */ |
||
| 377 | 1 | protected function assign_form_data($data) |
|
| 378 | { |
||
| 379 | 1 | $this->template->assign_vars(array( |
|
| 380 | 1 | 'AD_NAME' => $data['ad_name'], |
|
| 381 | 1 | 'AD_NOTE' => $data['ad_note'], |
|
| 382 | 1 | 'AD_CODE' => $data['ad_code'], |
|
| 383 | 1 | 'AD_ENABLED' => $data['ad_enabled'], |
|
| 384 | 1 | )); |
|
| 385 | 1 | } |
|
| 386 | |||
| 387 | /** |
||
| 388 | * Print success message. |
||
| 389 | * |
||
| 390 | * It takes arguments in the form of a language key, followed by language substitution values. |
||
| 391 | */ |
||
| 392 | protected function success() |
||
| 396 | |||
| 397 | /** |
||
| 398 | * Print error message. |
||
| 399 | * |
||
| 400 | * It takes arguments in the form of a language key, followed by language substitution values. |
||
| 401 | */ |
||
| 402 | protected function error() |
||
| 406 | } |
||
| 407 |
Loading history...
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.