Complex classes like Node 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 Node, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 45 | abstract class Node implements \Sabre\DAV\INode { |
||
| 46 | |||
| 47 | /** |
||
| 48 | * @var \OC\Files\View |
||
| 49 | */ |
||
| 50 | protected $fileView; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * The path to the current node |
||
| 54 | * |
||
| 55 | * @var string |
||
| 56 | */ |
||
| 57 | protected $path; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * node properties cache |
||
| 61 | * |
||
| 62 | * @var array |
||
| 63 | */ |
||
| 64 | protected $property_cache = null; |
||
| 65 | |||
| 66 | /** |
||
| 67 | * @var \OCP\Files\FileInfo |
||
| 68 | */ |
||
| 69 | protected $info; |
||
| 70 | |||
| 71 | /** |
||
| 72 | * @var IManager |
||
| 73 | */ |
||
| 74 | protected $shareManager; |
||
| 75 | |||
| 76 | /** |
||
| 77 | * Sets up the node, expects a full path name |
||
| 78 | * |
||
| 79 | * @param \OC\Files\View $view |
||
| 80 | * @param \OCP\Files\FileInfo $info |
||
| 81 | * @param IManager $shareManager |
||
| 82 | */ |
||
| 83 | public function __construct(View $view, FileInfo $info, IManager $shareManager = null) { |
||
| 93 | |||
| 94 | protected function refreshInfo() { |
||
| 97 | |||
| 98 | /** |
||
| 99 | * Returns the name of the node |
||
| 100 | * |
||
| 101 | * @return string |
||
| 102 | */ |
||
| 103 | public function getName() { |
||
| 106 | |||
| 107 | /** |
||
| 108 | * Returns the full path |
||
| 109 | * |
||
| 110 | * @return string |
||
| 111 | */ |
||
| 112 | public function getPath() { |
||
| 115 | |||
| 116 | /** |
||
| 117 | * Renames the node |
||
| 118 | * |
||
| 119 | * @param string $name The new name |
||
| 120 | * @throws \Sabre\DAV\Exception\BadRequest |
||
| 121 | * @throws \Sabre\DAV\Exception\Forbidden |
||
| 122 | */ |
||
| 123 | public function setName($name) { |
||
| 144 | |||
| 145 | public function setPropertyCache($property_cache) { |
||
| 148 | |||
| 149 | /** |
||
| 150 | * Returns the last modification time, as a unix timestamp |
||
| 151 | * |
||
| 152 | * @return int timestamp as integer |
||
| 153 | */ |
||
| 154 | public function getLastModified() { |
||
| 161 | |||
| 162 | /** |
||
| 163 | * sets the last modification time of the file (mtime) to the value given |
||
| 164 | * in the second parameter or to now if the second param is empty. |
||
| 165 | * Even if the modification time is set to a custom value the access time is set to now. |
||
| 166 | */ |
||
| 167 | public function touch($mtime) { |
||
| 171 | |||
| 172 | /** |
||
| 173 | * Returns the ETag for a file |
||
| 174 | * |
||
| 175 | * An ETag is a unique identifier representing the current version of the |
||
| 176 | * file. If the file changes, the ETag MUST change. The ETag is an |
||
| 177 | * arbitrary string, but MUST be surrounded by double-quotes. |
||
| 178 | * |
||
| 179 | * Return null if the ETag can not effectively be determined |
||
| 180 | * |
||
| 181 | * @return string |
||
| 182 | */ |
||
| 183 | public function getETag() { |
||
| 186 | |||
| 187 | /** |
||
| 188 | * Sets the ETag |
||
| 189 | * |
||
| 190 | * @param string $etag |
||
| 191 | * |
||
| 192 | * @return int file id of updated file or -1 on failure |
||
| 193 | */ |
||
| 194 | public function setETag($etag) { |
||
| 197 | |||
| 198 | /** |
||
| 199 | * Returns the size of the node, in bytes |
||
| 200 | * |
||
| 201 | * @return integer |
||
| 202 | */ |
||
| 203 | public function getSize() { |
||
| 206 | |||
| 207 | /** |
||
| 208 | * Returns the cache's file id |
||
| 209 | * |
||
| 210 | * @return int |
||
| 211 | */ |
||
| 212 | public function getId() { |
||
| 215 | |||
| 216 | /** |
||
| 217 | * @return string|null |
||
| 218 | */ |
||
| 219 | public function getFileId() { |
||
| 228 | |||
| 229 | /** |
||
| 230 | * @return integer |
||
| 231 | */ |
||
| 232 | public function getInternalFileId() { |
||
| 235 | |||
| 236 | /** |
||
| 237 | * @param string $user |
||
| 238 | * @return int |
||
| 239 | */ |
||
| 240 | public function getSharePermissions($user) { |
||
| 288 | |||
| 289 | /** |
||
| 290 | * @return string |
||
| 291 | */ |
||
| 292 | public function getDavPermissions() { |
||
| 323 | |||
| 324 | public function getOwner() { |
||
| 327 | |||
| 328 | protected function verifyPath() { |
||
| 336 | |||
| 337 | /** |
||
| 338 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 339 | */ |
||
| 340 | public function acquireLock($type) { |
||
| 343 | |||
| 344 | /** |
||
| 345 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 346 | */ |
||
| 347 | public function releaseLock($type) { |
||
| 350 | |||
| 351 | /** |
||
| 352 | * @param int $type \OCP\Lock\ILockingProvider::LOCK_SHARED or \OCP\Lock\ILockingProvider::LOCK_EXCLUSIVE |
||
| 353 | */ |
||
| 354 | public function changeLock($type) { |
||
| 357 | |||
| 358 | public function getFileInfo() { |
||
| 361 | } |
||
| 362 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountIdthat can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theidproperty of an instance of theAccountclass. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.