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 Blog_Controller 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 Blog_Controller, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 596 | class Blog_Controller extends Page_Controller {
|
||
| 597 | /** |
||
| 598 | * @var array |
||
| 599 | */ |
||
| 600 | private static $allowed_actions = array( |
||
| 601 | 'archive', |
||
| 602 | 'tag', |
||
| 603 | 'category', |
||
| 604 | 'rss', |
||
| 605 | 'profile', |
||
| 606 | ); |
||
| 607 | |||
| 608 | /** |
||
| 609 | * @var array |
||
| 610 | */ |
||
| 611 | private static $url_handlers = array( |
||
| 612 | 'tag/$Tag!' => 'tag', |
||
| 613 | 'category/$Category!' => 'category', |
||
| 614 | 'archive/$Year!/$Month/$Day' => 'archive', |
||
| 615 | 'profile/$URLSegment!' => 'profile', |
||
| 616 | ); |
||
| 617 | |||
| 618 | /** |
||
| 619 | * @var array |
||
| 620 | */ |
||
| 621 | private static $casting = array( |
||
| 622 | 'MetaTitle' => 'Text', |
||
| 623 | 'FilterDescription' => 'Text', |
||
| 624 | ); |
||
| 625 | |||
| 626 | /** |
||
| 627 | * The current Blog Post DataList query. |
||
| 628 | * |
||
| 629 | * @var DataList |
||
| 630 | */ |
||
| 631 | protected $blogPosts; |
||
| 632 | |||
| 633 | /** |
||
| 634 | * @return string |
||
| 635 | */ |
||
| 636 | public function index() {
|
||
| 637 | /** |
||
| 638 | * @var Blog $dataRecord |
||
| 639 | */ |
||
| 640 | $dataRecord = $this->dataRecord; |
||
| 641 | |||
| 642 | $this->blogPosts = $dataRecord->getBlogPosts(); |
||
| 643 | |||
| 644 | return $this->render(); |
||
| 645 | } |
||
| 646 | |||
| 647 | /** |
||
| 648 | * Renders a Blog Member's profile. |
||
| 649 | * |
||
| 650 | * @return SS_HTTPResponse |
||
| 651 | */ |
||
| 652 | public function profile() {
|
||
| 653 | $profile = $this->getCurrentProfile(); |
||
| 654 | |||
| 655 | if(!$profile) {
|
||
| 656 | return $this->httpError(404, 'Not Found'); |
||
| 657 | } |
||
| 658 | |||
| 659 | $this->blogPosts = $this->getCurrentProfilePosts(); |
||
| 660 | |||
| 661 | return $this->render(); |
||
| 662 | } |
||
| 663 | |||
| 664 | /** |
||
| 665 | * Get the Member associated with the current URL segment. |
||
| 666 | * |
||
| 667 | * @return null|Member |
||
| 668 | */ |
||
| 669 | public function getCurrentProfile() {
|
||
| 670 | $urlSegment = $this->request->param('URLSegment');
|
||
| 671 | |||
| 672 | if($urlSegment) {
|
||
| 673 | return Member::get() |
||
| 674 | ->filter('URLSegment', $urlSegment)
|
||
| 675 | ->first(); |
||
| 676 | } |
||
| 677 | |||
| 678 | return null; |
||
| 679 | } |
||
| 680 | |||
| 681 | /** |
||
| 682 | * Get posts related to the current Member profile. |
||
| 683 | * |
||
| 684 | * @return null|DataList |
||
| 685 | */ |
||
| 686 | public function getCurrentProfilePosts() {
|
||
| 687 | $profile = $this->getCurrentProfile(); |
||
| 688 | |||
| 689 | if($profile) {
|
||
| 690 | return $profile->BlogPosts()->filter('ParentID', $this->ID);
|
||
| 691 | } |
||
| 692 | |||
| 693 | return null; |
||
| 694 | } |
||
| 695 | |||
| 696 | /** |
||
| 697 | * Renders an archive for a specified date. This can be by year or year/month. |
||
| 698 | * |
||
| 699 | * @return null|SS_HTTPResponse |
||
| 700 | */ |
||
| 701 | public function archive() {
|
||
| 702 | /** |
||
| 703 | * @var Blog $dataRecord |
||
| 704 | */ |
||
| 705 | $dataRecord = $this->dataRecord; |
||
| 706 | |||
| 707 | $year = $this->getArchiveYear(); |
||
| 708 | $month = $this->getArchiveMonth(); |
||
| 709 | $day = $this->getArchiveDay(); |
||
| 710 | |||
| 711 | if($this->request->param('Month') && !$month) {
|
||
| 712 | $this->httpError(404, 'Not Found'); |
||
| 713 | } |
||
| 714 | |||
| 715 | if($month && $this->request->param('Day') && !$day) {
|
||
| 716 | $this->httpError(404, 'Not Found'); |
||
| 717 | } |
||
| 718 | |||
| 719 | if($year) {
|
||
| 720 | $this->blogPosts = $dataRecord->getArchivedBlogPosts($year, $month, $day); |
||
| 721 | |||
| 722 | return $this->render(); |
||
| 723 | } |
||
| 724 | |||
| 725 | $this->httpError(404, 'Not Found'); |
||
| 726 | |||
| 727 | return null; |
||
| 728 | } |
||
| 729 | |||
| 730 | /** |
||
| 731 | * Fetches the archive year from the url. |
||
| 732 | * |
||
| 733 | * @return int |
||
| 734 | */ |
||
| 735 | public function getArchiveYear() {
|
||
| 748 | |||
| 749 | /** |
||
| 750 | * Fetches the archive money from the url. |
||
| 751 | * |
||
| 752 | * @return null|int |
||
| 753 | */ |
||
| 754 | public function getArchiveMonth() {
|
||
| 755 | $month = $this->request->param('Month');
|
||
| 756 | |||
| 757 | if(preg_match('/^[0-9]{1,2}$/', $month)) {
|
||
| 758 | if($month > 0 && $month < 13) {
|
||
| 759 | if(checkdate($month, 01, $this->getArchiveYear())) {
|
||
| 760 | return (int) $month; |
||
| 761 | } |
||
| 762 | } |
||
| 763 | } |
||
| 764 | |||
| 765 | return null; |
||
| 766 | } |
||
| 767 | |||
| 768 | /** |
||
| 769 | * Fetches the archive day from the url. |
||
| 770 | * |
||
| 771 | * @return null|int |
||
| 772 | */ |
||
| 773 | public function getArchiveDay() {
|
||
| 774 | $day = $this->request->param('Day');
|
||
| 775 | |||
| 776 | if(preg_match('/^[0-9]{1,2}$/', $day)) {
|
||
| 777 | if(checkdate($this->getArchiveMonth(), $day, $this->getArchiveYear())) {
|
||
| 778 | return (int) $day; |
||
| 779 | } |
||
| 780 | } |
||
| 781 | |||
| 782 | return null; |
||
| 783 | } |
||
| 784 | |||
| 785 | /** |
||
| 786 | * Renders the blog posts for a given tag. |
||
| 787 | * |
||
| 788 | * @return null|SS_HTTPResponse |
||
| 789 | */ |
||
| 790 | View Code Duplication | public function tag() {
|
|
| 791 | $tag = $this->getCurrentTag(); |
||
| 792 | |||
| 793 | if($tag) {
|
||
| 794 | $this->blogPosts = $tag->BlogPosts(); |
||
| 795 | return $this->render(); |
||
| 796 | } |
||
| 797 | |||
| 798 | $this->httpError(404, 'Not Found'); |
||
| 799 | |||
| 800 | return null; |
||
| 801 | } |
||
| 802 | |||
| 803 | /** |
||
| 804 | * Tag Getter for use in templates. |
||
| 805 | * |
||
| 806 | * @return null|BlogTag |
||
| 807 | */ |
||
| 808 | View Code Duplication | public function getCurrentTag() {
|
|
| 809 | /** |
||
| 810 | * @var Blog $dataRecord |
||
| 811 | */ |
||
| 812 | $dataRecord = $this->dataRecord; |
||
| 813 | $tag = $this->request->param('Tag');
|
||
| 814 | if($tag) {
|
||
| 815 | return $dataRecord->Tags() |
||
| 816 | ->filter('URLSegment', array($tag, rawurlencode($tag)))
|
||
| 817 | ->first(); |
||
| 818 | } |
||
| 819 | return null; |
||
| 820 | } |
||
| 821 | |||
| 822 | /** |
||
| 823 | * Renders the blog posts for a given category. |
||
| 824 | * |
||
| 825 | * @return null|SS_HTTPResponse |
||
| 826 | */ |
||
| 827 | View Code Duplication | public function category() {
|
|
| 828 | $category = $this->getCurrentCategory(); |
||
| 829 | |||
| 830 | if($category) {
|
||
| 831 | $this->blogPosts = $category->BlogPosts(); |
||
| 832 | |||
| 833 | return $this->render(); |
||
| 834 | } |
||
| 835 | |||
| 836 | $this->httpError(404, 'Not Found'); |
||
| 837 | |||
| 838 | return null; |
||
| 839 | } |
||
| 840 | |||
| 841 | /** |
||
| 842 | * Category Getter for use in templates. |
||
| 843 | * |
||
| 844 | * @return null|BlogCategory |
||
| 845 | */ |
||
| 846 | View Code Duplication | public function getCurrentCategory() {
|
|
| 847 | /** |
||
| 848 | * @var Blog $dataRecord |
||
| 849 | */ |
||
| 850 | $dataRecord = $this->dataRecord; |
||
| 851 | $category = $this->request->param('Category');
|
||
| 852 | if($category) {
|
||
| 853 | return $dataRecord->Categories() |
||
| 854 | ->filter('URLSegment', array($category, rawurlencode($category)))
|
||
| 855 | ->first(); |
||
| 856 | } |
||
| 857 | return null; |
||
| 858 | } |
||
| 859 | |||
| 860 | /** |
||
| 861 | * Get the meta title for the current action. |
||
| 862 | * |
||
| 863 | * @return string |
||
| 864 | */ |
||
| 865 | public function getMetaTitle() {
|
||
| 866 | $title = $this->data()->getTitle(); |
||
| 867 | $filter = $this->getFilterDescription(); |
||
| 868 | |||
| 869 | if($filter) {
|
||
| 870 | $title = sprintf('%s - %s', $title, $filter);
|
||
| 871 | } |
||
| 872 | |||
| 873 | $this->extend('updateMetaTitle', $title);
|
||
| 874 | |||
| 875 | return $title; |
||
| 876 | } |
||
| 877 | |||
| 878 | /** |
||
| 879 | * Returns a description of the current filter. |
||
| 880 | * |
||
| 881 | * @return string |
||
| 882 | */ |
||
| 883 | public function getFilterDescription() {
|
||
| 884 | $items = array(); |
||
| 885 | |||
| 886 | $list = $this->PaginatedList(); |
||
| 887 | $currentPage = $list->CurrentPage(); |
||
| 888 | |||
| 889 | if($currentPage > 1) {
|
||
| 890 | $items[] = _t( |
||
| 891 | 'Blog.FILTERDESCRIPTION_PAGE', |
||
| 892 | 'Page {page}',
|
||
| 893 | null, |
||
| 894 | array( |
||
| 895 | 'page' => $currentPage, |
||
| 896 | ) |
||
| 897 | ); |
||
| 898 | } |
||
| 899 | |||
| 900 | if($author = $this->getCurrentProfile()) {
|
||
| 901 | $items[] = _t( |
||
| 902 | 'Blog.FILTERDESCRIPTION_AUTHOR', |
||
| 903 | 'By {author}',
|
||
| 904 | null, |
||
| 905 | array( |
||
| 906 | 'author' => $author->Title, |
||
| 907 | ) |
||
| 908 | ); |
||
| 909 | } |
||
| 910 | |||
| 911 | if($tag = $this->getCurrentTag()) {
|
||
| 912 | $items[] = _t( |
||
| 913 | 'Blog.FILTERDESCRIPTION_TAG', |
||
| 914 | 'Tagged with {tag}',
|
||
| 915 | null, |
||
| 916 | array( |
||
| 917 | 'tag' => $tag->Title, |
||
| 918 | ) |
||
| 919 | ); |
||
| 920 | } |
||
| 921 | |||
| 922 | if($category = $this->getCurrentCategory()) {
|
||
| 923 | $items[] = _t( |
||
| 924 | 'Blog.FILTERDESCRIPTION_CATEGORY', |
||
| 925 | 'In category {category}',
|
||
| 926 | null, |
||
| 927 | array( |
||
| 928 | 'category' => $category->Title, |
||
| 929 | ) |
||
| 930 | ); |
||
| 931 | } |
||
| 932 | |||
| 933 | if($this->owner->getArchiveYear()) {
|
||
| 934 | if($this->owner->getArchiveDay()) {
|
||
| 935 | $date = $this->owner->getArchiveDate()->Nice(); |
||
| 936 | } elseif($this->owner->getArchiveMonth()) {
|
||
| 937 | $date = $this->owner->getArchiveDate()->format('F, Y');
|
||
| 938 | } else {
|
||
| 939 | $date = $this->owner->getArchiveDate()->format('Y');
|
||
| 940 | } |
||
| 941 | |||
| 942 | $items[] = _t( |
||
| 943 | 'Blog.FILTERDESCRIPTION_DATE', |
||
| 944 | 'In {date}',
|
||
| 945 | null, |
||
| 946 | array( |
||
| 947 | 'date' => $date, |
||
| 948 | ) |
||
| 949 | ); |
||
| 950 | } |
||
| 951 | |||
| 952 | $result = ''; |
||
| 953 | |||
| 954 | if($items) {
|
||
| 955 | $result = implode(', ', $items);
|
||
| 956 | } |
||
| 957 | |||
| 958 | $this->extend('updateFilterDescription', $result);
|
||
| 959 | |||
| 960 | return $result; |
||
| 961 | } |
||
| 962 | |||
| 963 | /** |
||
| 964 | * Returns a list of paginated blog posts based on the BlogPost dataList. |
||
| 965 | * |
||
| 966 | * @return PaginatedList |
||
| 967 | */ |
||
| 968 | public function PaginatedList() {
|
||
| 969 | $allPosts = $this->blogPosts ?: new ArrayList(); |
||
| 970 | |||
| 971 | $posts = new PaginatedList($allPosts); |
||
| 972 | |||
| 973 | // Set appropriate page size |
||
| 974 | if($this->PostsPerPage > 0) {
|
||
| 975 | $pageSize = $this->PostsPerPage; |
||
| 976 | } elseif($count = $allPosts->count()) {
|
||
| 977 | $pageSize = $count; |
||
| 978 | } else {
|
||
| 979 | $pageSize = 99999; |
||
| 980 | } |
||
| 981 | $posts->setPageLength($pageSize); |
||
| 982 | |||
| 983 | // Set current page |
||
| 984 | $start = $this->request->getVar($posts->getPaginationGetVar()); |
||
| 985 | $posts->setPageStart($start); |
||
| 986 | |||
| 987 | return $posts; |
||
| 988 | } |
||
| 989 | |||
| 990 | /** |
||
| 991 | * Displays an RSS feed of blog posts. |
||
| 992 | * |
||
| 993 | * @return string |
||
| 994 | */ |
||
| 995 | public function rss() {
|
||
| 996 | /** |
||
| 997 | * @var Blog $dataRecord |
||
| 998 | */ |
||
| 999 | $dataRecord = $this->dataRecord; |
||
| 1000 | |||
| 1001 | $this->blogPosts = $dataRecord->getBlogPosts(); |
||
| 1002 | |||
| 1003 | $rss = new RSSFeed($this->blogPosts, $this->Link(), $this->MetaTitle, $this->MetaDescription); |
||
| 1004 | |||
| 1005 | $this->extend('updateRss', $rss);
|
||
| 1006 | |||
| 1007 | return $rss->outputToBrowser(); |
||
| 1008 | } |
||
| 1009 | |||
| 1010 | /** |
||
| 1011 | * Returns the current archive date. |
||
| 1012 | * |
||
| 1013 | * @return null|Date |
||
| 1014 | */ |
||
| 1015 | public function getArchiveDate() {
|
||
| 1016 | $year = $this->getArchiveYear(); |
||
| 1017 | $month = $this->getArchiveMonth(); |
||
| 1018 | $day = $this->getArchiveDay(); |
||
| 1019 | |||
| 1020 | if($year) {
|
||
| 1021 | if($month) {
|
||
| 1022 | $date = sprintf('%s-%s-01', $year, $month);
|
||
| 1023 | |||
| 1024 | if($day) {
|
||
| 1025 | $date = sprintf('%s-%s-%s', $year, $month, $day);
|
||
| 1026 | } |
||
| 1027 | } else {
|
||
| 1028 | $date = sprintf('%s-01-01', $year);
|
||
| 1029 | } |
||
| 1030 | |||
| 1031 | return DBField::create_field('Date', $date);
|
||
| 1032 | } |
||
| 1033 | |||
| 1034 | return null; |
||
| 1035 | } |
||
| 1036 | |||
| 1037 | /** |
||
| 1038 | * Returns a link to the RSS feed. |
||
| 1039 | * |
||
| 1040 | * @return string |
||
| 1041 | */ |
||
| 1042 | public function getRSSLink() {
|
||
| 1045 | } |
||
| 1046 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.