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:
| 1 | <?php |
||
| 9 | class FoxyStripeProduct extends DataObject implements PermissionProvider |
||
| 10 | { |
||
| 11 | |||
| 12 | /** |
||
| 13 | * @var string |
||
| 14 | */ |
||
| 15 | private static $singular_name = 'Product'; |
||
| 16 | /** |
||
| 17 | * @var string |
||
| 18 | */ |
||
| 19 | private static $plural_name = 'Products'; |
||
| 20 | /** |
||
| 21 | * @var string |
||
| 22 | */ |
||
| 23 | private static $description = 'A product that can be added to the shopping cart'; |
||
| 24 | |||
| 25 | /** |
||
| 26 | * @var array |
||
| 27 | */ |
||
| 28 | private static $db = [ |
||
| 29 | 'Title' => 'Varchar(255)', |
||
| 30 | 'Content' => 'HTMLText', |
||
| 31 | 'Price' => 'Currency', |
||
| 32 | 'Weight' => 'Decimal', |
||
| 33 | 'Code' => 'Varchar(100)', |
||
| 34 | 'ReceiptTitle' => 'HTMLVarchar(255)', |
||
| 35 | 'Featured' => 'Boolean', |
||
| 36 | 'Available' => 'Boolean', |
||
| 37 | 'DiscountTitle' => 'Varchar(50)' |
||
| 38 | ]; |
||
| 39 | |||
| 40 | /** |
||
| 41 | * @var array |
||
| 42 | */ |
||
| 43 | private static $has_one = [ |
||
| 44 | 'PreviewImage' => 'Image', |
||
| 45 | 'Category' => 'FoxyCartProductCategory' |
||
| 46 | ]; |
||
| 47 | |||
| 48 | /** |
||
| 49 | * @var array |
||
| 50 | */ |
||
| 51 | private static $has_many = [ |
||
| 52 | 'ProductImages' => 'ProductImage', |
||
| 53 | 'ProductOptions' => 'OptionItem', |
||
| 54 | 'OrderDetails' => 'OrderDetail', |
||
| 55 | 'ProductDiscountTiers' => 'ProductDiscountTier' |
||
| 56 | ]; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * @var array |
||
| 60 | */ |
||
| 61 | private static $belongs_many_many = [ |
||
| 62 | 'ProductHolders' => 'ProductHolder' |
||
| 63 | ]; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * @var array |
||
| 67 | */ |
||
| 68 | private static $indexes = [ |
||
| 69 | 'Code' => true, |
||
| 70 | ]; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * @var array |
||
| 74 | */ |
||
| 75 | private static $defaults = [ |
||
| 76 | 'ShowInMenus' => false, |
||
| 77 | 'Available' => true, |
||
| 78 | 'Weight' => '1.0' |
||
| 79 | ]; |
||
| 80 | |||
| 81 | /** |
||
| 82 | * @var array |
||
| 83 | */ |
||
| 84 | private static $summary_fields = [ |
||
| 85 | 'Title', |
||
| 86 | 'Code', |
||
| 87 | 'Price.Nice', |
||
| 88 | 'Category.Title' |
||
| 89 | ]; |
||
| 90 | |||
| 91 | /** |
||
| 92 | * @var array |
||
| 93 | */ |
||
| 94 | private static $searchable_fields = [ |
||
| 95 | 'Title', |
||
| 96 | 'Code', |
||
| 97 | 'Featured', |
||
| 98 | 'Available', |
||
| 99 | 'Category.ID' |
||
| 100 | ]; |
||
| 101 | |||
| 102 | public function fieldLabels($includeRelations = true) |
||
| 116 | |||
| 117 | /** |
||
| 118 | * @return FieldList |
||
| 119 | */ |
||
| 120 | public function getCMSFields() |
||
| 264 | |||
| 265 | 20 | public function onBeforeWrite() |
|
| 266 | { |
||
| 267 | 20 | parent::onBeforeWrite(); |
|
| 268 | 20 | if (!$this->CategoryID) { |
|
| 269 | $default = FoxyCartProductCategory::get()->filter(['Code' => 'DEFAULT'])->first(); |
||
| 270 | $this->CategoryID = $default->ID; |
||
| 271 | } |
||
| 272 | |||
| 273 | //update many_many lists when multi-group is on |
||
| 274 | 20 | if (SiteConfig::current_site_config()->MultiGroup) { |
|
| 275 | $holders = $this->ProductHolders(); |
||
| 276 | $product = ProductPage::get()->byID($this->ID); |
||
| 277 | if (isset($product->ParentID)) { |
||
| 278 | $origParent = $product->ParentID; |
||
| 279 | } else { |
||
| 280 | $origParent = null; |
||
| 281 | } |
||
| 282 | $currentParent = $this->ParentID; |
||
| 283 | if ($origParent != $currentParent) { |
||
| 284 | if ($holders->find('ID', $origParent)) { |
||
| 285 | $holders->removeByID($origParent); |
||
| 286 | } |
||
| 287 | |||
| 288 | } |
||
| 289 | $holders->add($currentParent); |
||
| 290 | } |
||
| 291 | |||
| 292 | 20 | $title = ltrim($this->Title); |
|
| 293 | 20 | $title = rtrim($title); |
|
| 294 | 20 | $this->Title = $title; |
|
| 295 | |||
| 296 | |||
| 297 | 20 | } |
|
| 298 | |||
| 299 | 20 | public function onAfterWrite() |
|
| 300 | { |
||
| 301 | 20 | parent::onAfterWrite(); |
|
| 302 | |||
| 303 | |||
| 304 | 20 | } |
|
| 305 | |||
| 306 | 1 | public function onBeforeDelete() |
|
| 307 | { |
||
| 308 | 1 | if ($this->Status != "Published") { |
|
| 309 | 1 | if ($this->ProductOptions()) { |
|
| 310 | 1 | $options = $this->getComponents('ProductOptions'); |
|
| 311 | 1 | foreach ($options as $option) { |
|
| 312 | $option->delete(); |
||
| 313 | 1 | } |
|
| 314 | 1 | } |
|
| 315 | 1 | if ($this->ProductImages()) { |
|
| 316 | //delete product image dataobjects, not the images themselves. |
||
| 317 | 1 | $images = $this->getComponents('ProductImages'); |
|
| 318 | 1 | foreach ($images as $image) { |
|
| 319 | $image->delete(); |
||
| 320 | 1 | } |
|
| 321 | 1 | } |
|
| 322 | 1 | } |
|
| 323 | 1 | parent::onBeforeDelete(); |
|
| 324 | 1 | } |
|
| 325 | |||
| 326 | 7 | public function validate() |
|
| 327 | { |
||
| 328 | 7 | $result = parent::validate(); |
|
| 329 | |||
| 330 | /*if($this->ID>0){ |
||
| 331 | if($this->Price <= 0) { |
||
| 332 | $result->error('Must set a positive price value'); |
||
| 333 | } |
||
| 334 | if($this->Weight <= 0){ |
||
| 335 | $result->error('Must set a positive weight value'); |
||
| 336 | } |
||
| 337 | if($this->Code == ''){ |
||
| 338 | $result->error('Must set a product code'); |
||
| 339 | } |
||
| 340 | }*/ |
||
| 341 | |||
| 342 | 7 | return $result; |
|
| 343 | } |
||
| 344 | |||
| 345 | public function getCMSValidator() |
||
| 349 | |||
| 350 | public static function getGeneratedValue($productCode = null, $optionName = null, $optionValue = null, $method = 'name', $output = false, $urlEncode = false) |
||
| 357 | |||
| 358 | // get FoxyCart Store Name for JS call |
||
| 359 | public function getCartScript() |
||
| 363 | |||
| 364 | public function getDiscountFieldValue() |
||
| 373 | |||
| 374 | /** |
||
| 375 | * @param Member $member |
||
| 376 | * @return boolean |
||
| 377 | */ |
||
| 378 | public function canEdit($member = null) |
||
| 382 | |||
| 383 | public function canDelete($member = null) |
||
| 387 | |||
| 388 | public function canCreate($member = null) |
||
| 392 | |||
| 393 | public function canPublish($member = null) |
||
| 397 | |||
| 398 | public function providePermissions() |
||
| 404 | |||
| 405 | /** |
||
| 406 | * @return FoxyStripePurchaseForm |
||
| 407 | */ |
||
| 408 | public function getPurchaseForm() |
||
| 420 | |||
| 421 | } |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the interface: