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 SystemAnnouncementManager 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 SystemAnnouncementManager, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 7 | class SystemAnnouncementManager |
||
| 8 | { |
||
| 9 | const VISIBLE_GUEST = 'visible_guest'; |
||
| 10 | const VISIBLE_STUDENT = 'visible_student'; |
||
| 11 | const VISIBLE_TEACHER = 'visible_teacher'; |
||
| 12 | // Requires DB change |
||
| 13 | const VISIBLE_DRH = 'visible_drh'; |
||
| 14 | const VISIBLE_SESSION_ADMIN = 'visible_session_admin'; |
||
| 15 | const VISIBLE_STUDENT_BOSS = 'visible_boss'; |
||
| 16 | |||
| 17 | /** |
||
| 18 | * @return array |
||
| 19 | */ |
||
| 20 | public static function getVisibilityList() |
||
| 38 | |||
| 39 | /** |
||
| 40 | * @param string $visibility |
||
| 41 | * @return string |
||
| 42 | */ |
||
| 43 | public static function getVisibilityCondition($visibility) |
||
| 53 | |||
| 54 | /** |
||
| 55 | * Displays all announcements |
||
| 56 | * @param string $visibility VISIBLE_GUEST, VISIBLE_STUDENT or VISIBLE_TEACHER |
||
| 57 | * @param int $id The identifier of the announcement to display |
||
| 58 | */ |
||
| 59 | public static function display_announcements($visibility, $id = -1) |
||
| 131 | |||
| 132 | /** |
||
| 133 | * @param string $visibility |
||
| 134 | * @param int $id |
||
| 135 | * @param int $start |
||
| 136 | * @param string $user_id |
||
| 137 | * @return string |
||
| 138 | */ |
||
| 139 | public static function displayAllAnnouncements($visibility, $id = -1, $start = 0, $user_id = '') |
||
| 222 | |||
| 223 | /** |
||
| 224 | * @param int $user_id |
||
| 225 | * @return string |
||
| 226 | */ |
||
| 227 | public static function display_arrow($user_id) |
||
| 246 | |||
| 247 | /** |
||
| 248 | * @param int $start |
||
| 249 | * @param string $user_id |
||
| 250 | * @return int |
||
| 251 | */ |
||
| 252 | public static function count_nb_announcement($start = 0, $user_id = '') |
||
| 277 | |||
| 278 | /** |
||
| 279 | * Get all announcements |
||
| 280 | * @return array An array with all available system announcements (as php |
||
| 281 | * objects) |
||
| 282 | */ |
||
| 283 | public static function get_all_announcements() |
||
| 304 | |||
| 305 | /** |
||
| 306 | * Adds an announcement to the database |
||
| 307 | * |
||
| 308 | * @param string $title Title of the announcement |
||
| 309 | * @param string $content Content of the announcement |
||
| 310 | * @param string $date_start Start date (YYYY-MM-DD HH:II: SS) |
||
| 311 | * @param string $date_end End date (YYYY-MM-DD HH:II: SS) |
||
| 312 | * @param array $visibility |
||
| 313 | * @param string $lang The language for which the announvement should be shown. Leave null for all langages |
||
| 314 | * @param int $send_mail Whether to send an e-mail to all users (1) or not (0) |
||
| 315 | * @param bool $add_to_calendar |
||
| 316 | * @param bool $sendEmailTest |
||
| 317 | * |
||
| 318 | * @return mixed insert_id on success, false on failure |
||
| 319 | */ |
||
| 320 | public static function add_announcement( |
||
| 321 | $title, |
||
| 322 | $content, |
||
| 323 | $date_start, |
||
| 324 | $date_end, |
||
| 325 | $visibility, |
||
| 326 | $lang = '', |
||
| 327 | $send_mail = 0, |
||
| 328 | $add_to_calendar = false, |
||
| 329 | $sendEmailTest = false |
||
| 330 | ) { |
||
| 331 | $original_content = $content; |
||
| 332 | $a_dateS = explode(' ', $date_start); |
||
| 333 | $a_arraySD = explode('-', $a_dateS[0]); |
||
| 334 | $a_arraySH = explode(':', $a_dateS[1]); |
||
| 335 | $date_start_to_compare = array_merge($a_arraySD, $a_arraySH); |
||
| 336 | |||
| 337 | $a_dateE = explode(' ', $date_end); |
||
| 338 | $a_arrayED = explode('-', $a_dateE[0]); |
||
| 339 | $a_arrayEH = explode(':', $a_dateE[1]); |
||
| 340 | $date_end_to_compare = array_merge($a_arrayED, $a_arrayEH); |
||
| 341 | |||
| 342 | $db_table = Database::get_main_table(TABLE_MAIN_SYSTEM_ANNOUNCEMENTS); |
||
| 343 | |||
| 344 | View Code Duplication | if (!checkdate($date_start_to_compare[1], $date_start_to_compare[2], $date_start_to_compare[0])) { |
|
| 345 | Display::addFlash(Display::return_message(get_lang('InvalidStartDate'), 'warning')); |
||
| 346 | |||
| 347 | return false; |
||
| 348 | } |
||
| 349 | |||
| 350 | View Code Duplication | if (($date_end_to_compare[1] || |
|
| 351 | $date_end_to_compare[2] || |
||
| 352 | $date_end_to_compare[0]) && |
||
| 353 | !checkdate($date_end_to_compare[1], $date_end_to_compare[2], $date_end_to_compare[0]) |
||
| 354 | ) { |
||
| 355 | Display::addFlash(Display::return_message(get_lang('InvalidEndDate'), 'warning')); |
||
| 356 | |||
| 357 | return false; |
||
| 358 | } |
||
| 359 | |||
| 360 | if (strlen(trim($title)) == 0) { |
||
| 361 | Display::addFlash(Display::return_message(get_lang('InvalidTitle'), 'warning')); |
||
| 362 | |||
| 363 | return false; |
||
| 364 | } |
||
| 365 | |||
| 366 | $start = api_get_utc_datetime($date_start); |
||
| 367 | $end = api_get_utc_datetime($date_end); |
||
| 368 | |||
| 369 | //Fixing urls that are sent by email |
||
| 370 | //$content = str_replace('src=\"/home/', 'src=\"'.api_get_path(WEB_PATH).'home/', $content); |
||
| 371 | //$content = str_replace('file=/home/', 'file='.api_get_path(WEB_PATH).'home/', $content); |
||
| 372 | $content = str_replace('src=\"'.api_get_path(REL_HOME_PATH), 'src=\"'.api_get_path(WEB_PATH).api_get_path(REL_HOME_PATH), $content); |
||
| 373 | $content = str_replace('file='.api_get_path(REL_HOME_PATH), 'file='.api_get_path(WEB_PATH).api_get_path(REL_HOME_PATH), $content); |
||
| 374 | $lang = is_null($lang) ? '' : $lang; |
||
| 375 | |||
| 376 | $current_access_url_id = 1; |
||
| 377 | if (api_is_multiple_url_enabled()) { |
||
| 378 | $current_access_url_id = api_get_current_access_url_id(); |
||
| 379 | } |
||
| 380 | |||
| 381 | $params = [ |
||
| 382 | 'title' => $title, |
||
| 383 | 'content' => $content, |
||
| 384 | 'date_start' => $start, |
||
| 385 | 'date_end' => $end, |
||
| 386 | 'lang' => $lang, |
||
| 387 | 'access_url_id' => $current_access_url_id |
||
| 388 | ]; |
||
| 389 | |||
| 390 | foreach ($visibility as $key => $value) { |
||
| 391 | $params[$key] = $value; |
||
| 392 | } |
||
| 393 | |||
| 394 | $resultId = Database::insert($db_table, $params); |
||
| 395 | |||
| 396 | if ($resultId) { |
||
| 397 | View Code Duplication | if ($sendEmailTest) { |
|
| 398 | self::send_system_announcement_by_email( |
||
| 399 | $title, |
||
| 400 | $content, |
||
| 401 | $visibility, |
||
| 402 | $lang, |
||
| 403 | true |
||
| 404 | ); |
||
| 405 | } else { |
||
| 406 | if ($send_mail == 1) { |
||
| 407 | self::send_system_announcement_by_email( |
||
| 408 | $title, |
||
| 409 | $content, |
||
| 410 | $visibility, |
||
| 411 | $lang |
||
| 412 | ); |
||
| 413 | } |
||
| 414 | } |
||
| 415 | |||
| 416 | if ($add_to_calendar) { |
||
| 417 | $agenda = new Agenda('admin'); |
||
| 418 | $agenda->addEvent( |
||
| 419 | $date_start, |
||
| 420 | $date_end, |
||
| 421 | false, |
||
| 422 | $title, |
||
| 423 | $original_content |
||
| 424 | ); |
||
| 425 | } |
||
| 426 | |||
| 427 | return $resultId; |
||
| 428 | |||
| 429 | } |
||
| 430 | |||
| 431 | return false; |
||
| 432 | } |
||
| 433 | |||
| 434 | /** |
||
| 435 | * Makes the announcement id visible only for groups in groups_array |
||
| 436 | * @param int $announcement_id |
||
| 437 | * @param array $group_array array of group id |
||
| 438 | * @return bool |
||
| 439 | **/ |
||
| 440 | public static function announcement_for_groups($announcement_id, $group_array) |
||
| 469 | |||
| 470 | /** |
||
| 471 | * Gets the groups of this announce |
||
| 472 | * @param int announcement id |
||
| 473 | * @return array array of group id |
||
| 474 | **/ |
||
| 475 | public static function get_announcement_groups($announcement_id) |
||
| 476 | { |
||
| 477 | $tbl_announcement_group = Database::get_main_table(TABLE_MAIN_SYSTEM_ANNOUNCEMENTS_GROUPS); |
||
| 478 | $tbl_group = Database::get_main_table(TABLE_USERGROUP); |
||
| 479 | //first delete all group associations for this announcement |
||
| 480 | $sql = "SELECT |
||
| 481 | g.id as group_id, |
||
| 482 | g.name as group_name |
||
| 483 | FROM $tbl_group g , $tbl_announcement_group ag |
||
| 484 | WHERE |
||
| 485 | announcement_id =".intval($announcement_id)." AND |
||
| 486 | ag.group_id = g.id"; |
||
| 487 | $res = Database::query($sql); |
||
| 488 | $groups = Database::fetch_array($res); |
||
| 489 | |||
| 490 | return $groups; |
||
| 491 | } |
||
| 492 | |||
| 493 | /** |
||
| 494 | * Updates an announcement to the database |
||
| 495 | * |
||
| 496 | * @param integer $id of the announcement |
||
| 497 | * @param string $title title of the announcement |
||
| 498 | * @param string $content content of the announcement |
||
| 499 | * @param array $date_start start date (0 => day ; 1 => month ; 2 => year ; 3 => hour ; 4 => minute) |
||
| 500 | * @param array $date_end end date of (0 => day ; 1 => month ; 2 => year ; 3 => hour ; 4 => minute) |
||
| 501 | * @param array $visibility |
||
| 502 | * @return bool True on success, false on failure |
||
| 503 | */ |
||
| 504 | public static function update_announcement( |
||
| 505 | $id, |
||
| 506 | $title, |
||
| 507 | $content, |
||
| 508 | $date_start, |
||
| 509 | $date_end, |
||
| 510 | $visibility, |
||
| 511 | $lang = null, |
||
| 512 | $send_mail = 0, |
||
| 513 | $sendEmailTest = false |
||
| 514 | ) { |
||
| 515 | $em = Database::getManager(); |
||
| 516 | $announcement = $em->find('ChamiloCoreBundle:SysAnnouncement', $id); |
||
| 517 | if (!$announcement) { |
||
| 518 | return false; |
||
| 519 | } |
||
| 520 | |||
| 521 | $a_dateS = explode(' ', $date_start); |
||
| 522 | $a_arraySD = explode('-', $a_dateS[0]); |
||
| 523 | $a_arraySH = explode(':', $a_dateS[1]); |
||
| 524 | $date_start_to_compare = array_merge($a_arraySD, $a_arraySH); |
||
| 525 | |||
| 526 | $a_dateE = explode(' ', $date_end); |
||
| 527 | $a_arrayED = explode('-', $a_dateE[0]); |
||
| 528 | $a_arrayEH = explode(':', $a_dateE[1]); |
||
| 529 | $date_end_to_compare = array_merge($a_arrayED, $a_arrayEH); |
||
| 530 | |||
| 531 | $lang = is_null($lang) ? '' : $lang; |
||
| 532 | |||
| 533 | View Code Duplication | if (!checkdate($date_start_to_compare[1], $date_start_to_compare[2], $date_start_to_compare[0])) { |
|
| 534 | echo Display::return_message(get_lang('InvalidStartDate')); |
||
| 535 | |||
| 536 | return false; |
||
| 537 | } |
||
| 538 | |||
| 539 | View Code Duplication | if (($date_end_to_compare[1] || |
|
| 540 | $date_end_to_compare[2] || |
||
| 541 | $date_end_to_compare[0]) && |
||
| 542 | !checkdate($date_end_to_compare[1], $date_end_to_compare[2], $date_end_to_compare[0]) |
||
| 543 | ) { |
||
| 544 | echo Display::return_message(get_lang('InvalidEndDate')); |
||
| 545 | |||
| 546 | return false; |
||
| 547 | } |
||
| 548 | |||
| 549 | if (strlen(trim($title)) == 0) { |
||
| 550 | echo Display::return_message(get_lang('InvalidTitle')); |
||
| 551 | |||
| 552 | return false; |
||
| 553 | } |
||
| 554 | |||
| 555 | $start = api_get_utc_datetime($date_start); |
||
| 556 | $end = api_get_utc_datetime($date_end); |
||
| 557 | |||
| 558 | //Fixing urls that are sent by email |
||
| 559 | //$content = str_replace('src=\"/home/', 'src=\"'.api_get_path(WEB_PATH).'home/', $content); |
||
| 560 | //$content = str_replace('file=/home/', 'file='.api_get_path(WEB_PATH).'home/', $content); |
||
| 561 | $content = str_replace('src=\"'.api_get_path(REL_HOME_PATH), 'src=\"'.api_get_path(WEB_PATH).api_get_path(REL_HOME_PATH), $content); |
||
| 562 | $content = str_replace('file='.api_get_path(REL_HOME_PATH), 'file='.api_get_path(WEB_PATH).api_get_path(REL_HOME_PATH), $content); |
||
| 563 | |||
| 564 | View Code Duplication | if ($sendEmailTest) { |
|
| 565 | self::send_system_announcement_by_email( |
||
| 566 | $title, |
||
| 567 | $content, |
||
| 568 | null, |
||
| 569 | null, |
||
| 570 | $lang, |
||
| 571 | $sendEmailTest |
||
| 572 | ); |
||
| 573 | } else { |
||
| 574 | if ($send_mail == 1) { |
||
| 575 | self::send_system_announcement_by_email( |
||
| 576 | $title, |
||
| 577 | $content, |
||
| 578 | $visibility, |
||
| 579 | $lang |
||
| 580 | ); |
||
| 581 | } |
||
| 582 | } |
||
| 583 | |||
| 584 | $dateStart = new DateTime($start, new DateTimeZone('UTC')); |
||
| 585 | $dateEnd = new DateTime($end, new DateTimeZone('UTC')); |
||
| 586 | |||
| 587 | $announcement |
||
| 588 | ->setLang($lang) |
||
| 589 | ->setTitle($title) |
||
| 590 | ->setContent($content) |
||
| 591 | ->setDateStart($dateStart) |
||
| 592 | ->setDateEnd($dateEnd) |
||
| 593 | //->setVisibleTeacher($visible_teacher) |
||
| 594 | //->setVisibleStudent($visible_student) |
||
| 595 | //->setVisibleGuest($visible_guest) |
||
| 596 | ->setAccessUrlId(api_get_current_access_url_id()); |
||
| 597 | |||
| 598 | $em->merge($announcement); |
||
| 599 | $em->flush(); |
||
| 600 | |||
| 601 | // Update visibility |
||
| 602 | $list = self::getVisibilityList(); |
||
| 603 | $table = Database::get_main_table(TABLE_MAIN_SYSTEM_ANNOUNCEMENTS); |
||
| 604 | foreach ($list as $key => $title) { |
||
| 605 | $value = isset($visibility[$key]) && $visibility[$key] ? 1 : 0; |
||
| 606 | $sql = "UPDATE $table SET $key = '$value' WHERE id = $id"; |
||
| 607 | Database::query($sql); |
||
| 608 | } |
||
| 609 | return true; |
||
| 610 | } |
||
| 611 | |||
| 612 | /** |
||
| 613 | * Deletes an announcement |
||
| 614 | * @param int $id The identifier of the announcement that should be |
||
| 615 | * @return bool True on success, false on failure |
||
| 616 | */ |
||
| 617 | public static function delete_announcement($id) |
||
| 628 | |||
| 629 | /** |
||
| 630 | * Gets an announcement |
||
| 631 | * @param int $id The identifier of the announcement that should be |
||
| 632 | * @return object Object of class StdClass or the required class, containing the query result row |
||
| 633 | */ |
||
| 634 | public static function get_announcement($id) |
||
| 643 | |||
| 644 | /** |
||
| 645 | * Change the visibility of an announcement |
||
| 646 | * @param int $id |
||
| 647 | * @param int $user For who should the visibility be changed |
||
| 648 | * @param bool $visible |
||
| 649 | * @return bool True on success, false on failure |
||
| 650 | */ |
||
| 651 | public static function set_visibility($id, $user, $visible) |
||
| 673 | |||
| 674 | /** |
||
| 675 | * Send a system announcement by e-mail to all teachers/students depending on parameters |
||
| 676 | * @param string $title |
||
| 677 | * @param string $content |
||
| 678 | * @param array $visibility |
||
| 679 | * @param string $language Language (optional, considered for all languages if left empty) |
||
| 680 | * @param bool $sendEmailTest |
||
| 681 | * @return bool True if the message was sent or there was no destination matching. |
||
| 682 | * False on database or e-mail sending error. |
||
| 683 | */ |
||
| 684 | public static function send_system_announcement_by_email( |
||
| 763 | |||
| 764 | /** |
||
| 765 | * Displays announcements as an slideshow |
||
| 766 | * @param string $visible see self::VISIBLE_* constants |
||
| 767 | * @param int $id The identifier of the announcement to display |
||
| 768 | * |
||
| 769 | * @return string |
||
| 770 | */ |
||
| 771 | public static function displayAnnouncementsSlider($visible, $id = null) |
||
| 829 | |||
| 830 | /** |
||
| 831 | * Get the HTML code for an announcement |
||
| 832 | * @param int $announcementId The announcement ID |
||
| 833 | * @param int $visibility The announcement visibility |
||
| 834 | * @return string The HTML code |
||
| 835 | */ |
||
| 836 | public static function displayAnnouncement($announcementId, $visibility) |
||
| 871 | |||
| 872 | /** |
||
| 873 | * @return bool |
||
| 874 | */ |
||
| 875 | public static function newRolesActivated() |
||
| 885 | |||
| 886 | /** |
||
| 887 | * @return string |
||
| 888 | */ |
||
| 889 | public static function getCurrentUserVisibility() |
||
| 918 | } |
||
| 919 |
This check looks for accesses to local static members using the fully qualified name instead of
self::.While this is perfectly valid, the fully qualified name of
Certificate::TRIPLEDES_CBCcould just as well be replaced byself::TRIPLEDES_CBC. Referencing local members withself::assured the access will still work when the class is renamed, makes it perfectly clear that the member is in fact local and will usually be shorter.