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.