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 FileFormFactory 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 FileFormFactory, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
18 | class FileFormFactory extends AssetFormFactory |
||
19 | { |
||
20 | protected function getFormFieldTabs($record, $context = []) |
||
21 | { |
||
22 | // Add extra tab |
||
23 | $tabs = TabSet::create( |
||
24 | 'Editor', |
||
25 | $this->getFormFieldDetailsTab($record, $context), |
||
26 | $this->getFormFieldSecurityTab($record, $context), |
||
27 | $this->getFormFieldUsageTab($record, $context), |
||
28 | $this->getFormFieldHistoryTab($record, $context) |
||
29 | ); |
||
30 | |||
31 | // All non-admin forms are typically readonly |
||
32 | switch ($this->getFormType($context)) { |
||
33 | case static::TYPE_INSERT_MEDIA: |
||
34 | case static::TYPE_UPDATE_MEDIA: |
||
35 | $tabs->setReadonly(true); |
||
36 | $tabs->unshift($this->getFormFieldAttributesTab($record, $context)); |
||
37 | break; |
||
38 | case static::TYPE_INSERT_LINK: |
||
39 | case static::TYPE_UPDATE_LINK: |
||
40 | $tabs->setReadonly(true); |
||
41 | $tabs->unshift($this->getFormFieldLinkOptionsTab($record, $context)); |
||
42 | break; |
||
43 | case static::TYPE_SELECT: |
||
44 | $tabs->setReadonly(true); |
||
45 | break; |
||
46 | } |
||
47 | |||
48 | return $tabs; |
||
49 | } |
||
50 | |||
51 | /** |
||
52 | * Build "Usage" tab |
||
53 | * |
||
54 | * @param File $record |
||
55 | * @param array $context |
||
56 | * @return Tab |
||
57 | */ |
||
58 | protected function getFormFieldUsageTab($record, $context = []) |
||
59 | { |
||
60 | // Add new tab for usage |
||
61 | return Tab::create( |
||
62 | 'Usage', |
||
63 | DatetimeField::create( |
||
64 | "Created", |
||
65 | _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.CREATED', 'First uploaded') |
||
66 | ) |
||
67 | ->setReadonly(true), |
||
68 | DatetimeField::create( |
||
69 | "LastEdited", |
||
70 | _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.LASTEDIT', 'Last changed') |
||
71 | ) |
||
72 | ->setReadonly(true) |
||
73 | ); |
||
74 | } |
||
75 | |||
76 | protected function getFormFieldDetailsTab($record, $context = []) |
||
77 | { |
||
78 | // Update details tab |
||
79 | $tab = parent::getFormFieldDetailsTab($record, $context); |
||
80 | |||
81 | $tab->insertBefore('Name', TextField::create("Title", File::singleton()->fieldLabel('Title'))); |
||
82 | |||
83 | if ($this->getFormType($context) !== static::TYPE_ADMIN) { |
||
84 | $tab->push(LiteralField::create( |
||
85 | 'EditLink', |
||
86 | sprintf( |
||
87 | '<a href="%s" class="%s" target="_blank"><i class="%s" />%s</a>', |
||
88 | $record->CMSEditLink(), |
||
89 | 'btn btn-secondary-outline font-icon-edit editor__edit-link', |
||
90 | '', |
||
91 | _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.EditLink', 'Edit original file') |
||
92 | ) |
||
93 | )); |
||
94 | } |
||
95 | return $tab; |
||
96 | } |
||
97 | |||
98 | protected function getFormFieldLinkOptionsTab($record, $context = []) |
||
99 | { |
||
100 | return Tab::create( |
||
101 | 'LinkOptions', |
||
102 | _t(__CLASS__ .'.LINKOPTIONS', 'Link options'), |
||
103 | TextField::create( |
||
104 | 'Description', |
||
105 | _t(__CLASS__.'.LINKDESCR', 'Link description') |
||
106 | ), |
||
107 | CheckboxField::create( |
||
108 | 'TargetBlank', |
||
109 | _t(__CLASS__.'.LINKOPENNEWWIN', 'Open in new window/tab') |
||
110 | ) |
||
111 | ); |
||
112 | } |
||
113 | |||
114 | /** |
||
115 | * Create tab for file attributes |
||
116 | * |
||
117 | * @param File $record |
||
118 | * @param array $context |
||
119 | * @return Tab |
||
120 | */ |
||
121 | protected function getFormFieldAttributesTab($record, $context = []) |
||
122 | { |
||
123 | return Tab::create( |
||
124 | 'Placement', |
||
125 | LiteralField::create( |
||
126 | 'AttributesDescription', |
||
127 | '<p>'. _t( |
||
128 | 'SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.AttributesDescription', |
||
129 | 'These changes will only affect this particular placement of the file.' |
||
130 | ) .'</p>' |
||
131 | ), |
||
132 | TextField::create('Caption', _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.Caption', 'Caption')) |
||
133 | ); |
||
134 | } |
||
135 | |||
136 | protected function getFormFieldHistoryTab($record, $context = []) |
||
137 | { |
||
138 | return Tab::create( |
||
139 | 'History', |
||
140 | HistoryListField::create('HistoryList') |
||
141 | ->setRecord($record) |
||
142 | ); |
||
143 | } |
||
144 | |||
145 | protected function getFormFields(RequestHandler $controller = null, $formName, $context = []) |
||
146 | { |
||
147 | /** @var File $record */ |
||
148 | $record = $context['Record']; |
||
149 | |||
150 | // Add status flag before extensions are triggered |
||
151 | $this->beforeExtending('updateFormFields', function (FieldList $fields) use ($record) { |
||
152 | // @todo move specs to a component/class, so it can update specs when a File is replaced |
||
153 | $fields->insertAfter( |
||
154 | 'TitleHeader', |
||
155 | LiteralField::create('FileSpecs', $this->getSpecsMarkup($record)) |
||
156 | ); |
||
157 | $fields->push(HiddenField::create('FileFilename')); |
||
158 | $fields->push(HiddenField::create('FileHash')); |
||
159 | $fields->push(HiddenField::create('FileVariant')); |
||
160 | }); |
||
161 | |||
162 | return parent::getFormFields($controller, $formName, $context); |
||
163 | } |
||
164 | |||
165 | /** |
||
166 | * Get publish action |
||
167 | * |
||
168 | * @param File $record |
||
169 | * @return FormAction |
||
170 | */ |
||
171 | protected function getPublishAction($record) |
||
172 | { |
||
173 | if (!$record || !$record->canPublish()) { |
||
174 | return null; |
||
175 | } |
||
176 | |||
177 | // Build action |
||
178 | $publishText = _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.PUBLISH_BUTTON', 'Publish'); |
||
179 | /** @var FormAction $action */ |
||
180 | $action = FormAction::create('publish', $publishText) |
||
181 | ->setIcon('rocket') |
||
182 | ->setSchemaData(['data' => ['buttonStyle' => 'primary']]); |
||
183 | |||
184 | return $action; |
||
185 | } |
||
186 | |||
187 | protected function getFormActions(RequestHandler $controller = null, $formName, $context = []) |
||
188 | { |
||
189 | $record = $context['Record']; |
||
190 | $type = $this->getFormType($context); |
||
191 | |||
192 | if ($type === static::TYPE_SELECT || $type === static::TYPE_INSERT_MEDIA) { |
||
193 | $actionItems = array_filter([ |
||
194 | $this->getInsertAction($record), |
||
195 | ]); |
||
196 | } elseif ($type === static::TYPE_UPDATE_MEDIA) { |
||
197 | $actionItems = array_filter([ |
||
198 | $this->getUpdateAction($record), |
||
199 | ]); |
||
200 | } elseif ($type === static::TYPE_INSERT_LINK) { |
||
201 | $actionItems = array_filter([ |
||
202 | $this->getInsertLinkAction($record), |
||
203 | ]); |
||
204 | } elseif ($type === static::TYPE_UPDATE_LINK) { |
||
205 | $actionItems = array_filter([ |
||
206 | $this->getUpdateLinkAction($record), |
||
207 | ]); |
||
208 | } else { |
||
209 | $actionItems = array_filter([ |
||
210 | $this->getSaveAction($record), |
||
211 | $this->getPublishAction($record) |
||
212 | ]); |
||
213 | } |
||
214 | |||
215 | // Group all actions |
||
216 | if (count($actionItems) > 1) { |
||
217 | $actionItems = [ |
||
218 | FieldGroup::create($actionItems) |
||
219 | ->setName('Actions') |
||
220 | ->addExtraClass('btn-group') |
||
221 | ]; |
||
222 | } |
||
223 | |||
224 | if ($type === static::TYPE_ADMIN) { |
||
225 | // Add popover |
||
226 | $popover = $this->getPopoverMenu($record); |
||
227 | if ($popover) { |
||
228 | $actionItems[] = $popover; |
||
229 | } |
||
230 | } |
||
231 | |||
232 | // Build |
||
233 | $actions = new FieldList($actionItems); |
||
234 | |||
235 | // Update |
||
236 | $this->invokeWithExtensions('updateFormActions', $actions, $controller, $formName, $context); |
||
237 | return $actions; |
||
238 | } |
||
239 | |||
240 | /** |
||
241 | * get HTML for status icon |
||
242 | * |
||
243 | * @param File $record |
||
244 | * @return null|string |
||
245 | */ |
||
246 | protected function getSpecsMarkup($record) |
||
247 | { |
||
248 | if (!$record || !$record->exists()) { |
||
249 | return null; |
||
250 | } |
||
251 | return sprintf( |
||
252 | '<div class="editor__specs">%s %s</div>', |
||
253 | $record->getSize(), |
||
254 | $this->getStatusFlagMarkup($record) |
||
255 | ); |
||
256 | } |
||
257 | |||
258 | /** |
||
259 | * Get published status flag |
||
260 | * |
||
261 | * @param File $record |
||
262 | * @return null|string |
||
263 | */ |
||
264 | protected function getStatusFlagMarkup($record) |
||
265 | { |
||
266 | if ($record && ($statusTitle = $record->getStatusTitle())) { |
||
267 | return "<span class=\"editor__status-flag\">{$statusTitle}</span>"; |
||
268 | } |
||
269 | return null; |
||
270 | } |
||
271 | |||
272 | /** |
||
273 | * Get action for publishing |
||
274 | * |
||
275 | * @param File $record |
||
276 | * @return FormAction |
||
277 | */ |
||
278 | protected function getUnpublishAction($record) |
||
293 | |||
294 | /** |
||
295 | * Get actions that go into the Popover menu |
||
296 | * |
||
297 | * @param $record |
||
298 | * @return array |
||
299 | */ |
||
300 | protected function getPopoverActions($record) |
||
301 | { |
||
302 | $this->beforeExtending('updatePopoverActions', function (&$actions, $record) { |
||
303 | // add the unpublish action to the start of the array |
||
304 | array_unshift($actions, $this->getUnpublishAction($record)); |
||
309 | |||
310 | /** |
||
311 | * @param File $record |
||
312 | * @return FormAction |
||
313 | */ |
||
314 | View Code Duplication | protected function getInsertAction($record) |
|
324 | |||
325 | /** |
||
326 | * @param File $record |
||
327 | * @return FormAction |
||
328 | */ |
||
329 | View Code Duplication | protected function getUpdateAction($record) |
|
339 | |||
340 | /** |
||
341 | * @param File $record |
||
342 | * @return FormAction |
||
343 | */ |
||
344 | View Code Duplication | protected function getInsertLinkAction($record) |
|
354 | |||
355 | /** |
||
356 | * @param File $record |
||
357 | * @return FormAction |
||
358 | */ |
||
359 | View Code Duplication | protected function getUpdateLinkAction($record) |
|
369 | } |
||
370 |
This check looks from parameters that have been defined for a function or method, but which are not used in the method body.