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 PageRow 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 PageRow, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
5 | class PageRow extends DataObject |
||
|
|||
6 | { |
||
7 | |||
8 | /** |
||
9 | * |
||
10 | * @var string |
||
11 | */ |
||
12 | private static $bg_image_prefix = ''; |
||
13 | |||
14 | |||
15 | /** |
||
16 | * |
||
17 | * @var string |
||
18 | */ |
||
19 | private static $bg_image_postfix = ''; |
||
20 | |||
21 | /** |
||
22 | * |
||
23 | * @var array |
||
24 | */ |
||
25 | private static $background_image_options = []; |
||
26 | |||
27 | /** |
||
28 | * |
||
29 | * @var array |
||
30 | */ |
||
31 | private static $background_style_options = []; |
||
32 | |||
33 | /** |
||
34 | * |
||
35 | * @var int |
||
36 | */ |
||
37 | private static $max_number_of_pages_for_tree_selector = 500; |
||
38 | |||
39 | ####################### |
||
40 | ### Names Section |
||
41 | ####################### |
||
42 | |||
43 | private static $singular_name = 'Content Block'; |
||
44 | |||
45 | public function i18n_singular_name() |
||
49 | |||
50 | private static $plural_name = 'PageRows'; |
||
51 | |||
52 | public function i18n_plural_name() |
||
56 | |||
57 | |||
58 | ####################### |
||
59 | ### Model Section |
||
60 | ####################### |
||
61 | |||
62 | private static $db = [ |
||
63 | 'ReadyForPublication' => 'Boolean', |
||
64 | 'Code' => 'Varchar(7)', |
||
65 | 'Title' => 'Varchar(255)', |
||
66 | 'BackgroundImage' => 'Varchar(255)', |
||
67 | 'BackgroundStyle' => 'Varchar(100)' |
||
68 | ]; |
||
69 | |||
70 | private static $belongs_many_many = [ |
||
71 | 'Pages' => 'Page' |
||
72 | ]; |
||
73 | |||
74 | |||
75 | ####################### |
||
76 | ### Further DB Field Details |
||
77 | ####################### |
||
78 | |||
79 | private static $indexes = [ |
||
80 | 'Created' => true, |
||
81 | 'Code' => true, |
||
82 | 'ReadyForPublication' => true |
||
83 | ]; |
||
84 | |||
85 | private static $default_sort = [ |
||
86 | 'ReadyForPublication' => 'DESC', |
||
87 | 'Created' => 'DESC' |
||
88 | ]; |
||
89 | |||
90 | private static $required_fields = [ |
||
91 | 'Title' |
||
92 | ]; |
||
93 | |||
94 | private static $searchable_fields = [ |
||
95 | 'ClassName' => 'ExactMatchFilter', |
||
96 | 'ReadyForPublication' => 'ExactMatchFilter', |
||
97 | 'Code' => 'PartialMatchFilter', |
||
98 | 'Title' => 'PartialMatchFilter' |
||
99 | ]; |
||
100 | |||
101 | public function scaffoldSearchFields($_params = null) { |
||
118 | |||
119 | ####################### |
||
120 | ### Field Names and Presentation Section |
||
121 | ####################### |
||
122 | |||
123 | private static $summary_fields = [ |
||
124 | 'CodeNice' => 'Code', |
||
125 | 'Type' => 'Type', |
||
126 | 'TitleStrong' => 'Title', |
||
127 | 'UsedOn' => 'Used On', |
||
128 | 'ReadyForPublicationStrong' => 'Published' |
||
129 | ]; |
||
130 | |||
131 | private static $field_labels = [ |
||
132 | 'ReadyForPublication' => 'Ready' |
||
133 | ]; |
||
134 | |||
135 | private static $field_labels_right = [ |
||
136 | 'Code' => 'This is auto-generated (unique) code to keep track of content blocks', |
||
137 | 'ReadyForPublication' => 'Is this Content Block ready for publication?' |
||
138 | ]; |
||
139 | |||
140 | |||
141 | ####################### |
||
142 | ### Casting Section |
||
143 | ####################### |
||
144 | |||
145 | private static $casting = [ |
||
146 | 'CodeNice' => 'HTMLText', |
||
147 | 'Type' => 'Varchar', |
||
148 | 'UsedOn' => 'Varchar', |
||
149 | 'ChildPageRowHTML' => 'HTMLText', |
||
150 | 'TitleStrong' => 'HTMLText', |
||
151 | 'ReadyForPublicationStrong' => 'HTMLText' |
||
152 | ]; |
||
153 | |||
154 | |||
155 | public function getType() |
||
159 | |||
160 | public function getCodeNice() |
||
166 | |||
167 | public function getUsedOn() |
||
177 | |||
178 | public function getTitleStrong() |
||
184 | |||
185 | public function getReadyForPublicationStrong() |
||
195 | |||
196 | |||
197 | |||
198 | ####################### |
||
199 | ### can Section |
||
200 | ####################### |
||
201 | |||
202 | /** |
||
203 | * only children can be created |
||
204 | */ |
||
205 | |||
206 | |||
207 | public function canDelete($member = null) |
||
211 | |||
212 | |||
213 | |||
214 | ####################### |
||
215 | ### write Section |
||
216 | ####################### |
||
217 | |||
218 | public function requireDefaultRecords() |
||
271 | |||
272 | |||
273 | public function validate() |
||
316 | |||
317 | public function onBeforeWrite() |
||
325 | |||
326 | public function onAfterWrite() |
||
336 | |||
337 | |||
338 | ####################### |
||
339 | ### Import / Export Section |
||
340 | ####################### |
||
341 | |||
342 | |||
343 | ####################### |
||
344 | ### CMS Edit Section |
||
345 | ####################### |
||
346 | |||
347 | |||
348 | protected $runCMSFieldFixups = true; |
||
349 | |||
350 | public function getCMSFields() |
||
508 | |||
509 | protected function myCMSFieldGroups() |
||
518 | |||
519 | ############################################# |
||
520 | ### TEMPLATE STUFF |
||
521 | ############################################# |
||
522 | |||
523 | |||
524 | public function HTMLClassNamesAsString() |
||
531 | |||
532 | public function ChildInheritedClassNamesAsString() |
||
536 | |||
537 | protected function baseClassesForHTMLAsArray() |
||
554 | |||
555 | protected function makeIntoHTMLClasses($array) |
||
564 | |||
565 | public function UserCanEditMe() |
||
569 | |||
570 | |||
571 | private static $_current_owner_page_object_overridden = null; |
||
572 | |||
573 | public static function set_current_page_object($page) |
||
577 | |||
578 | |||
579 | public function CurrentOwnerPageObject() |
||
589 | |||
590 | public function CurrentOwnerController() |
||
597 | |||
598 | public function AdditionalHTMLClassNamesAsArray() |
||
602 | |||
603 | public function HTMLOutputAlwaysOutput($includeJS = true) |
||
607 | |||
608 | |||
609 | public function HTMLOutput($includeJS = true, $alwaysShow = false) |
||
625 | |||
626 | /** |
||
627 | * custom scripts required to run this show |
||
628 | * @return array |
||
629 | */ |
||
630 | protected function customScripts() |
||
634 | |||
635 | protected function templateForHTMLOutput($className = null) |
||
642 | |||
643 | View Code Duplication | public function MyPreviewLink($id = 0) |
|
652 | |||
653 | View Code Duplication | public function MyPublicViewLink() |
|
661 | |||
662 | |||
663 | public function MoreDetailsRowChildLinkingID() |
||
667 | |||
668 | |||
669 | |||
670 | public function UserCanEditBlock($member = null) |
||
674 | |||
675 | public function ContextRelevantCMSEditLink() |
||
684 | |||
685 | |||
686 | public function ChildClassMethodName() |
||
690 | |||
691 | private $_childPageRowHTML = null; |
||
692 | |||
693 | /** |
||
694 | * @alias for HasChildPageRowHTML |
||
695 | * @return bool |
||
696 | */ |
||
697 | public function HasChild() |
||
701 | |||
702 | public function AsChildOpenByDefault() |
||
706 | |||
707 | |||
708 | public function MoreDetailsRowParentLinkingID() |
||
716 | |||
717 | |||
718 | public function CalculatedHasMoreDetailsRow() |
||
732 | |||
733 | /** |
||
734 | * |
||
735 | * @return bool |
||
736 | */ |
||
737 | public function HasChildPageRowHTML() |
||
741 | |||
742 | /** |
||
743 | * @return null / string |
||
744 | */ |
||
745 | public function ChildPageRowHTML() |
||
749 | |||
750 | /** |
||
751 | * @return null / string |
||
752 | */ |
||
753 | public function getChildPageRowHTML() |
||
767 | |||
768 | ############################### |
||
769 | # COPIED AND PATCHED FROM: |
||
770 | # https://github.com/jonom/silverstripe-version-history/blob/master/code/VersionHistoryExtension.php |
||
771 | ############################### |
||
772 | |||
773 | protected function getImageCMSFields() |
||
777 | |||
778 | protected function getLayoutCMSFields() |
||
782 | |||
783 | protected function getBackgroundFields() |
||
802 | |||
803 | |||
804 | protected function getPublishCMSFields() |
||
811 | |||
812 | private $_bgImage = null; |
||
813 | |||
814 | protected $_theSameBackgroundImageForAllOfThisTypeToDo = true; |
||
815 | |||
816 | public function IsPageRowWithBackgroundImage() |
||
827 | |||
828 | public function HasBackgroundImage() |
||
832 | |||
833 | public function CalculatedBackgroundImage() |
||
860 | |||
861 | protected function HasSameBackgroundImageForAllOfThisType() |
||
865 | |||
866 | protected function BackgroundImageOptions() |
||
870 | |||
871 | View Code Duplication | protected function BackgroundImageOptionsWithBasics() |
|
880 | |||
881 | |||
882 | protected $_bgStyle = null; |
||
883 | |||
884 | public function IsPageRowWithBackgroundStyle() |
||
891 | |||
892 | public function CalculatedBackgroundStyle() |
||
909 | |||
910 | |||
911 | View Code Duplication | protected function BackgroundStyleOptionsWithBasics() |
|
920 | |||
921 | |||
922 | protected function BackgroundStyleOptions() |
||
926 | } |
||
927 |
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.