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 SilverstripeColumnsPageExtension 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 SilverstripeColumnsPageExtension, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
9 | class SilverstripeColumnsPageExtension extends DataExtension |
||
|
|||
10 | { |
||
11 | private static $db = [ |
||
12 | 'Summary' => 'HTMLVarchar(255)', |
||
13 | 'DefaultSidebarContent' => 'HTMLText' |
||
14 | ]; |
||
15 | |||
16 | private static $has_one = [ |
||
17 | 'SummaryImage' => 'Image', |
||
18 | 'SidebarImage' => 'Image' |
||
19 | ]; |
||
20 | |||
21 | private static $casting = [ |
||
22 | 'MyDefaultSidebarContent' => 'HTMLText', |
||
23 | 'FullWidthContent' => 'HTMLText', |
||
24 | 'SummaryContent' => 'HTMLText' |
||
25 | ]; |
||
26 | |||
27 | private static $field_labels = [ |
||
28 | 'Summary' => 'Page Summary', |
||
29 | 'DefaultSidebarContent' => 'Sidebar content', |
||
30 | 'SummaryImage' => 'Image for Summaries', |
||
31 | 'SidebarImage' => 'Sidebar Image' |
||
32 | ]; |
||
33 | |||
34 | private static $field_labels_right = [ |
||
35 | 'Summary' => 'A summary of the page for use on other pages.', |
||
36 | 'DefaultSidebarContent' => 'The sidebar show up to the right of the main content. It is usually for something like DID YOU KNOW? or CONTACT DETAILS.', |
||
37 | 'SummaryImage' => 'Image used to show a link to this page together with the summary of the page provided.', |
||
38 | 'SidebarImage' => 'Image to show up in the sidebar instead of content.' |
||
39 | ]; |
||
40 | |||
41 | private static $page_types_that_use_the_default_sidebar = []; |
||
42 | |||
43 | private static $page_types_that_use_the_second_column = []; |
||
44 | |||
45 | public function updateCMSFields(FieldList $fields) |
||
83 | |||
84 | |||
85 | |||
86 | /** |
||
87 | * @return boolean |
||
88 | */ |
||
89 | function UseDefaultSideBarContent() |
||
110 | |||
111 | /** |
||
112 | * @return Image | null |
||
113 | */ |
||
114 | function MySidebarImage() |
||
136 | |||
137 | /** |
||
138 | * |
||
139 | * @return string (HTML) |
||
140 | */ |
||
141 | View Code Duplication | function getMyDefaultSidebarContent() |
|
151 | |||
152 | /** |
||
153 | * |
||
154 | * @return string (HTML) |
||
155 | */ |
||
156 | View Code Duplication | function getFullWidthContent() |
|
166 | |||
167 | /** |
||
168 | * |
||
169 | * @return string (HTML) |
||
170 | */ |
||
171 | View Code Duplication | function getSummaryContent() |
|
181 | |||
182 | private static $_children_show_in_menu = []; |
||
183 | |||
184 | private $showMenuItemsFor = null; |
||
185 | |||
186 | public function setShowMenuItemsFor($showMenuItemsFor) { |
||
190 | |||
191 | function ChildrenShowInMenu($root = false) |
||
221 | |||
222 | function MyMenuItems() |
||
257 | |||
258 | function MyMenuItemsParentPage() |
||
270 | |||
271 | function MyMenuItemsParentLink() |
||
279 | |||
280 | function MyMenuItemsMenuLink($id = null) |
||
287 | |||
288 | } |
||
289 |
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.