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 Common 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 Common, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 68 | abstract class Common implements Storage, ILockingStorage, IVersionedStorage { |
||
| 69 | use LocalTempFileTrait; |
||
| 70 | |||
| 71 | protected $cache; |
||
| 72 | protected $scanner; |
||
| 73 | protected $watcher; |
||
| 74 | protected $propagator; |
||
| 75 | protected $storageCache; |
||
| 76 | protected $updater; |
||
| 77 | |||
| 78 | protected $mountOptions = []; |
||
| 79 | protected $owner = null; |
||
| 80 | |||
| 81 | public function __construct($parameters) { |
||
| 83 | |||
| 84 | /** |
||
| 85 | * Remove a file or folder |
||
| 86 | * |
||
| 87 | * @param string $path |
||
| 88 | * @return bool |
||
| 89 | */ |
||
| 90 | View Code Duplication | protected function remove($path) { |
|
| 99 | |||
| 100 | public function is_dir($path) { |
||
| 103 | |||
| 104 | public function is_file($path) { |
||
| 107 | |||
| 108 | public function filesize($path) { |
||
| 120 | |||
| 121 | public function isReadable($path) { |
||
| 126 | |||
| 127 | public function isUpdatable($path) { |
||
| 133 | |||
| 134 | public function isCreatable($path) { |
||
| 140 | |||
| 141 | public function isDeletable($path) { |
||
| 148 | |||
| 149 | public function isSharable($path) { |
||
| 152 | |||
| 153 | public function getPermissions($path) { |
||
| 172 | |||
| 173 | public function filemtime($path) { |
||
| 181 | |||
| 182 | public function file_get_contents($path) { |
||
| 191 | |||
| 192 | public function file_put_contents($path, $data) { |
||
| 199 | |||
| 200 | public function rename($path1, $path2) { |
||
| 206 | |||
| 207 | public function copy($path1, $path2) { |
||
| 229 | |||
| 230 | public function getMimeType($path) { |
||
| 239 | |||
| 240 | View Code Duplication | public function hash($type, $path, $raw = false) { |
|
| 247 | |||
| 248 | public function search($query) { |
||
| 251 | |||
| 252 | public function getLocalFile($path) { |
||
| 255 | |||
| 256 | /** |
||
| 257 | * @param string $path |
||
| 258 | * @param string $target |
||
| 259 | */ |
||
| 260 | private function addLocalFolder($path, $target) { |
||
| 276 | |||
| 277 | /** |
||
| 278 | * @param string $query |
||
| 279 | * @param string $dir |
||
| 280 | * @return array |
||
| 281 | */ |
||
| 282 | protected function searchInDir($query, $dir = '') { |
||
| 301 | |||
| 302 | /** |
||
| 303 | * check if a file or folder has been updated since $time |
||
| 304 | * |
||
| 305 | * The method is only used to check if the cache needs to be updated. Storage backends that don't support checking |
||
| 306 | * the mtime should always return false here. As a result storage implementations that always return false expect |
||
| 307 | * exclusive access to the backend and will not pick up files that have been added in a way that circumvents |
||
| 308 | * ownClouds filesystem. |
||
| 309 | * |
||
| 310 | * @param string $path |
||
| 311 | * @param int $time |
||
| 312 | * @return bool |
||
| 313 | */ |
||
| 314 | public function hasUpdated($path, $time) { |
||
| 317 | |||
| 318 | public function getCache($path = '', $storage = null) { |
||
| 327 | |||
| 328 | public function getScanner($path = '', $storage = null) { |
||
| 337 | |||
| 338 | public function getWatcher($path = '', $storage = null) { |
||
| 349 | |||
| 350 | /** |
||
| 351 | * get a propagator instance for the cache |
||
| 352 | * |
||
| 353 | * @param \OC\Files\Storage\Storage (optional) the storage to pass to the watcher |
||
| 354 | * @return \OC\Files\Cache\Propagator |
||
| 355 | */ |
||
| 356 | public function getPropagator($storage = null) { |
||
| 365 | |||
| 366 | public function getUpdater($storage = null) { |
||
| 375 | |||
| 376 | public function getStorageCache($storage = null) { |
||
| 385 | |||
| 386 | /** |
||
| 387 | * get the owner of a path |
||
| 388 | * |
||
| 389 | * @param string $path The path to get the owner |
||
| 390 | * @return string|false uid or false |
||
| 391 | */ |
||
| 392 | public function getOwner($path) { |
||
| 399 | |||
| 400 | /** |
||
| 401 | * get the ETag for a file or folder |
||
| 402 | * |
||
| 403 | * @param string $path |
||
| 404 | * @return string |
||
| 405 | */ |
||
| 406 | public function getETag($path) { |
||
| 409 | |||
| 410 | /** |
||
| 411 | * clean a path, i.e. remove all redundant '.' and '..' |
||
| 412 | * making sure that it can't point to higher than '/' |
||
| 413 | * |
||
| 414 | * @param string $path The path to clean |
||
| 415 | * @return string cleaned path |
||
| 416 | */ |
||
| 417 | public function cleanPath($path) { |
||
| 433 | |||
| 434 | /** |
||
| 435 | * Test a storage for availability |
||
| 436 | * |
||
| 437 | * @return bool |
||
| 438 | */ |
||
| 439 | public function test() { |
||
| 445 | |||
| 446 | /** |
||
| 447 | * get the free space in the storage |
||
| 448 | * |
||
| 449 | * @param string $path |
||
| 450 | * @return int|false |
||
| 451 | */ |
||
| 452 | public function free_space($path) { |
||
| 455 | |||
| 456 | /** |
||
| 457 | * {@inheritdoc} |
||
| 458 | */ |
||
| 459 | public function isLocal() { |
||
| 464 | |||
| 465 | /** |
||
| 466 | * Check if the storage is an instance of $class or is a wrapper for a storage that is an instance of $class |
||
| 467 | * |
||
| 468 | * @param string $class |
||
| 469 | * @return bool |
||
| 470 | */ |
||
| 471 | public function instanceOfStorage($class) { |
||
| 478 | |||
| 479 | /** |
||
| 480 | * A custom storage implementation can return an url for direct download of a give file. |
||
| 481 | * |
||
| 482 | * For now the returned array can hold the parameter url - in future more attributes might follow. |
||
| 483 | * |
||
| 484 | * @param string $path |
||
| 485 | * @return array|false |
||
| 486 | */ |
||
| 487 | public function getDirectDownload($path) { |
||
| 490 | |||
| 491 | /** |
||
| 492 | * @inheritdoc |
||
| 493 | */ |
||
| 494 | public function verifyPath($path, $fileName) { |
||
| 501 | |||
| 502 | /** |
||
| 503 | * @param string $fileName |
||
| 504 | * @throws InvalidPathException |
||
| 505 | */ |
||
| 506 | protected function verifyPosixPath($fileName) { |
||
| 514 | |||
| 515 | /** |
||
| 516 | * @param string $fileName |
||
| 517 | * @param string $invalidChars |
||
| 518 | * @throws InvalidPathException |
||
| 519 | */ |
||
| 520 | private function scanForInvalidCharacters($fileName, $invalidChars) { |
||
| 532 | |||
| 533 | /** |
||
| 534 | * @param array $options |
||
| 535 | */ |
||
| 536 | public function setMountOptions(array $options) { |
||
| 539 | |||
| 540 | /** |
||
| 541 | * @param string $name |
||
| 542 | * @param mixed $default |
||
| 543 | * @return mixed |
||
| 544 | */ |
||
| 545 | public function getMountOption($name, $default = null) { |
||
| 548 | |||
| 549 | /** |
||
| 550 | * @param \OCP\Files\Storage $sourceStorage |
||
| 551 | * @param string $sourceInternalPath |
||
| 552 | * @param string $targetInternalPath |
||
| 553 | * @param bool $preserveMtime |
||
| 554 | * @return bool |
||
| 555 | */ |
||
| 556 | public function copyFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath, $preserveMtime = false) { |
||
| 595 | |||
| 596 | /** |
||
| 597 | * @param \OCP\Files\Storage $sourceStorage |
||
| 598 | * @param string $sourceInternalPath |
||
| 599 | * @param string $targetInternalPath |
||
| 600 | * @return bool |
||
| 601 | */ |
||
| 602 | public function moveFromStorage(\OCP\Files\Storage $sourceStorage, $sourceInternalPath, $targetInternalPath) { |
||
| 621 | |||
| 622 | /** |
||
| 623 | * @inheritdoc |
||
| 624 | */ |
||
| 625 | public function getMetaData($path) { |
||
| 649 | |||
| 650 | /** |
||
| 651 | * @param string $path |
||
| 652 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 653 | * @param \OCP\Lock\ILockingProvider $provider |
||
| 654 | * @throws \OCP\Lock\LockedException |
||
| 655 | */ |
||
| 656 | public function acquireLock($path, $type, ILockingProvider $provider) { |
||
| 659 | |||
| 660 | /** |
||
| 661 | * @param string $path |
||
| 662 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 663 | * @param \OCP\Lock\ILockingProvider $provider |
||
| 664 | */ |
||
| 665 | public function releaseLock($path, $type, ILockingProvider $provider) { |
||
| 668 | |||
| 669 | /** |
||
| 670 | * @param string $path |
||
| 671 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 672 | * @param \OCP\Lock\ILockingProvider $provider |
||
| 673 | * @throws \OCP\Lock\LockedException |
||
| 674 | */ |
||
| 675 | public function changeLock($path, $type, ILockingProvider $provider) { |
||
| 678 | |||
| 679 | /** |
||
| 680 | * @return array [ available, last_checked ] |
||
| 681 | */ |
||
| 682 | public function getAvailability() { |
||
| 685 | |||
| 686 | /** |
||
| 687 | * @param bool $isAvailable |
||
| 688 | */ |
||
| 689 | public function setAvailability($isAvailable) { |
||
| 705 | |||
| 706 | /** |
||
| 707 | * @param $internalPath |
||
| 708 | * @return array |
||
| 709 | */ |
||
| 710 | protected function convertInternalPathToGlobalPath($internalPath) { |
||
| 721 | |||
| 722 | public function getVersion($internalPath, $versionId) { |
||
| 729 | |||
| 730 | public function getContentOfVersion($internalPath, $versionId) { |
||
| 734 | |||
| 735 | public function restoreVersion($internalPath, $versionId) { |
||
| 743 | |||
| 744 | public function saveVersion($internalPath) { |
||
| 748 | } |
||
| 749 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.