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 AttachmentAwareTrait 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 AttachmentAwareTrait, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 36 | trait AttachmentAwareTrait |
||
| 37 | { |
||
| 38 | /** |
||
| 39 | * A store of cached attachments, by ID. |
||
| 40 | * |
||
| 41 | * @var Attachment[] $attachmentCache |
||
| 42 | */ |
||
| 43 | protected static $attachmentCache = []; |
||
| 44 | |||
| 45 | /** |
||
| 46 | * Store a collection of node objects. |
||
| 47 | * |
||
| 48 | * @var Collection|Attachment[] |
||
| 49 | */ |
||
| 50 | protected $attachments = []; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * Store the widget instance currently displaying attachments. |
||
| 54 | * |
||
| 55 | * @var AttachmentWidget |
||
| 56 | */ |
||
| 57 | protected $attachmentWidget; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * Retrieve the objects associated to the current object. |
||
| 61 | * |
||
| 62 | * @param array|string|null $group Filter the attachments by a group identifier. |
||
| 63 | * When an array, filter the attachments by a options list. |
||
| 64 | * @param string|null $type Filter the attachments by type. |
||
| 65 | * @param callable|null $before Process each attachment before applying data. |
||
| 66 | * @param callable|null $after Process each attachment after applying data. |
||
| 67 | * @throws InvalidArgumentException If the $group or $type is invalid. |
||
| 68 | * @return Collection|Attachment[] |
||
| 69 | */ |
||
| 70 | public function getAttachments( |
||
| 251 | |||
| 252 | /** |
||
| 253 | * Determine if the current object has any nodes. |
||
| 254 | * |
||
| 255 | * @return boolean Whether $this has any nodes (TRUE) or not (FALSE). |
||
| 256 | */ |
||
| 257 | public function hasAttachments() |
||
| 261 | |||
| 262 | /** |
||
| 263 | * Count the number of nodes associated to the current object. |
||
| 264 | * |
||
| 265 | * @return integer |
||
| 266 | */ |
||
| 267 | public function numAttachments() |
||
| 273 | |||
| 274 | /** |
||
| 275 | * Attach an node to the current object. |
||
| 276 | * |
||
| 277 | * @param AttachableInterface|ModelInterface $attachment An attachment or object. |
||
| 278 | * @param string $group Attachment group, defaults to contents. |
||
| 279 | * @return boolean|self |
||
| 280 | */ |
||
| 281 | public function addAttachment($attachment, $group = 'contents') |
||
| 302 | |||
| 303 | /** |
||
| 304 | * Remove all joins linked to a specific attachment. |
||
| 305 | * |
||
| 306 | * @deprecated in favour of AttachmentAwareTrait::removeAttachmentJoins() |
||
| 307 | * @return boolean |
||
| 308 | */ |
||
| 309 | public function removeJoins() |
||
| 319 | |||
| 320 | /** |
||
| 321 | * Remove all joins linked to a specific attachment. |
||
| 322 | * |
||
| 323 | * @return boolean |
||
| 324 | */ |
||
| 325 | public function removeAttachmentJoins() |
||
| 343 | |||
| 344 | /** |
||
| 345 | * Delete the objects associated to the current object. |
||
| 346 | * |
||
| 347 | * @param array $options Filter the attachments by an option list. |
||
| 348 | * @return boolean |
||
| 349 | */ |
||
| 350 | public function deleteAttachments(array $options = []) |
||
| 358 | |||
| 359 | /** |
||
| 360 | * Retrieve the attachment widget. |
||
| 361 | * |
||
| 362 | * @return AttachmentWidget |
||
| 363 | */ |
||
| 364 | protected function attachmentWidget() |
||
| 368 | |||
| 369 | /** |
||
| 370 | * Set the attachment widget. |
||
| 371 | * |
||
| 372 | * @param AttachmentWidget $widget The widget displaying attachments. |
||
| 373 | * @return string |
||
| 374 | */ |
||
| 375 | protected function setAttachmentWidget(AttachmentWidget $widget) |
||
| 381 | |||
| 382 | /** |
||
| 383 | * Available attachment obj_type related to the current object. |
||
| 384 | * This goes throught the entire forms / form groups, starting from the |
||
| 385 | * dashboard widgets. |
||
| 386 | * Returns an array of object classes by group |
||
| 387 | * [ |
||
| 388 | * group : [ |
||
| 389 | * 'object\type', |
||
| 390 | * 'object\type2', |
||
| 391 | * 'object\type3' |
||
| 392 | * ] |
||
| 393 | * ] |
||
| 394 | * @return array Attachment obj_types. |
||
| 395 | */ |
||
| 396 | public function attachmentObjTypes() |
||
| 443 | |||
| 444 | /** |
||
| 445 | * Parse a given options for loading a collection of attachments. |
||
| 446 | * |
||
| 447 | * @param array $options A list of options. |
||
| 448 | * Option keys not present in {@see self::getDefaultAttachmentOptions() default options} |
||
| 449 | * are rejected. |
||
| 450 | * @return array |
||
| 451 | */ |
||
| 452 | protected function parseAttachmentOptions(array $options) |
||
| 462 | |||
| 463 | /** |
||
| 464 | * Parse a given options for loading a collection of attachments. |
||
| 465 | * |
||
| 466 | * @param mixed $val The option value. |
||
| 467 | * @param string $key The option key. |
||
| 468 | * @return boolean Return TRUE if the value is preserved. Otherwise FALSE. |
||
| 469 | */ |
||
| 470 | protected function filterAttachmentOption($val, $key) |
||
| 487 | |||
| 488 | /** |
||
| 489 | * Retrieve the default options for loading a collection of attachments. |
||
| 490 | * |
||
| 491 | * @return array |
||
| 492 | */ |
||
| 493 | protected function getDefaultAttachmentOptions() |
||
| 503 | |||
| 504 | |||
| 505 | |||
| 506 | // Abstract Methods |
||
| 507 | // ========================================================================= |
||
| 508 | |||
| 509 | /** |
||
| 510 | * Retrieve the object's unique ID. |
||
| 511 | * |
||
| 512 | * @return mixed |
||
| 513 | */ |
||
| 514 | abstract public function id(); |
||
| 515 | |||
| 516 | /** |
||
| 517 | * Retrieve the object model factory. |
||
| 518 | * |
||
| 519 | * @return \Charcoal\Factory\FactoryInterface |
||
| 520 | */ |
||
| 521 | abstract public function modelFactory(); |
||
| 522 | |||
| 523 | /** |
||
| 524 | * Retrieve the model collection loader. |
||
| 525 | * |
||
| 526 | * @return \Charcoal\Loader\CollectionLoader |
||
| 527 | */ |
||
| 528 | abstract public function collectionLoader(); |
||
| 529 | } |
||
| 530 |
In PHP it is possible to write to properties without declaring them. For example, the following is perfectly valid PHP code:
Generally, it is a good practice to explictly declare properties to avoid accidental typos and provide IDE auto-completion: