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 CampaignMonitorCampaign 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 CampaignMonitorCampaign, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
9 | class CampaignMonitorCampaign extends DataObject |
||
|
|||
10 | { |
||
11 | |||
12 | /** |
||
13 | * |
||
14 | * @var array |
||
15 | */ |
||
16 | private static $emogrifier_add_allowed_media_types = array( |
||
17 | "screen" |
||
18 | ); |
||
19 | |||
20 | /** |
||
21 | * |
||
22 | * @var array |
||
23 | */ |
||
24 | private static $emogrifier_remove_allowed_media_types = array(); |
||
25 | |||
26 | /** |
||
27 | * |
||
28 | * @var string |
||
29 | */ |
||
30 | private static $default_template = "CampaignMonitorCampaign"; |
||
31 | |||
32 | private static $db = array( |
||
33 | "HasBeenSent" => "Boolean", |
||
34 | "MessageFromNewsletterServer" => "Text", |
||
35 | "CreateAsTemplate" => "Boolean", |
||
36 | "CreateFromWebsite" => "Boolean", |
||
37 | "CreatedFromWebsite" => "Boolean", |
||
38 | "TemplateID" => "Varchar(40)", |
||
39 | "CampaignID" => "Varchar(40)", |
||
40 | "Name" => "Varchar(100)", |
||
41 | "Subject" => "Varchar(100)", |
||
42 | "FromName" => "Varchar(100)", |
||
43 | "FromEmail" => "Varchar(100)", |
||
44 | "ReplyTo" => "Varchar(100)", |
||
45 | "SentDate" => "SS_Datetime", |
||
46 | "WebVersionURL" => "Varchar(255)", |
||
47 | "WebVersionTextURL" => "Varchar(255)", |
||
48 | "Hide" => "Boolean", |
||
49 | "Content" => "HTMLText", |
||
50 | "Hash" => "Varchar(32)" |
||
51 | ); |
||
52 | |||
53 | private static $indexes = array( |
||
54 | "TemplateID" => true, |
||
55 | "CampaignID" => true, |
||
56 | "Hide" => true, |
||
57 | "Hash" => true |
||
58 | ); |
||
59 | |||
60 | private static $field_labels = array( |
||
61 | "CreateFromWebsite" => "Create on newsletter server" |
||
62 | ); |
||
63 | |||
64 | private static $has_one = array( |
||
65 | "CampaignMonitorCampaignStyle" => "CampaignMonitorCampaignStyle" |
||
66 | ); |
||
67 | |||
68 | private static $many_many = array( |
||
69 | "Pages" => "CampaignMonitorSignupPage" |
||
70 | ); |
||
71 | |||
72 | private static $searchable_fields = array( |
||
73 | "Subject" => "PartialMatchFilter", |
||
74 | "Content" => "PartialMatchFilter", |
||
75 | "Hide" => "ExactMatch" |
||
76 | ); |
||
77 | |||
78 | private static $summary_fields = array( |
||
79 | "Subject" => "Subject", |
||
80 | "SentDate" => "Sent Date" |
||
81 | ); |
||
82 | |||
83 | private static $singular_name = "Campaign"; |
||
84 | |||
85 | private static $plural_name = "Campaigns"; |
||
86 | |||
87 | private static $default_sort = "Hide ASC, SentDate DESC"; |
||
88 | |||
89 | public function canDelete($member = null) |
||
93 | |||
94 | public function getCMSFields() |
||
157 | |||
158 | /** |
||
159 | * returns link to view campaign |
||
160 | * @var return |
||
161 | */ |
||
162 | View Code Duplication | public function Link($action = "") |
|
170 | |||
171 | /** |
||
172 | * returns link to view preview campaign |
||
173 | * this link is used to create templates / campaigns on Campaign Monitor |
||
174 | * @var return |
||
175 | */ |
||
176 | View Code Duplication | public function PreviewLink($action = "") |
|
184 | |||
185 | /** |
||
186 | * html for newsletter to be created |
||
187 | * @var return |
||
188 | */ |
||
189 | public function getNewsletterContent() |
||
229 | |||
230 | /** |
||
231 | * provide template used for RenderWith |
||
232 | * @return string |
||
233 | */ |
||
234 | public function getRenderWithTemplate() |
||
243 | |||
244 | /** |
||
245 | * @return array |
||
246 | */ |
||
247 | protected function getCSSFileLocations() |
||
254 | |||
255 | /** |
||
256 | * @return array |
||
257 | */ |
||
258 | public function getHTMLContent() |
||
265 | |||
266 | protected $countOfWrites = 0; |
||
267 | |||
268 | public function onBeforeWrite() |
||
282 | |||
283 | public function onAfterWrite() |
||
305 | |||
306 | public function onBeforeDelete() |
||
322 | |||
323 | private static $_api = null; |
||
324 | |||
325 | /** |
||
326 | * |
||
327 | * @return CampaignMonitorAPIConnector |
||
328 | */ |
||
329 | protected function getAPI() |
||
337 | |||
338 | private $_hasBeenSent = null; |
||
339 | |||
340 | public function HasBeenSentCheck() |
||
369 | |||
370 | private $_existsOnCampaignMonitorCheck = null; |
||
371 | |||
372 | /** |
||
373 | * checks if the template and/or the campaign exists |
||
374 | * @return boolean |
||
375 | */ |
||
376 | public function ExistsOnCampaignMonitorCheck($forceRecheck = false) |
||
423 | } |
||
424 |
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.