| Conditions | 4 |
| Paths | 7 |
| Total Lines | 70 |
| Code Lines | 47 |
| Lines | 0 |
| Ratio | 0 % |
| Changes | 1 | ||
| Bugs | 0 | Features | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
| 1 | <?php |
||
| 100 | public function moveFileForArea( |
||
| 101 | int $fileId, |
||
| 102 | int $cid, |
||
| 103 | ?int $sid, |
||
| 104 | int $uid, |
||
| 105 | int $targetCatId, |
||
| 106 | string $area |
||
| 107 | ): int { |
||
| 108 | $conn = $this->getEntityManager()->getConnection(); |
||
| 109 | $targetCatId = (int) $targetCatId; |
||
| 110 | |||
| 111 | if ($area === 'sent') { |
||
| 112 | // Move inside sender's space: update file's category if current user is the uploader. |
||
| 113 | $sql = <<<SQL |
||
| 114 | UPDATE c_dropbox_file |
||
| 115 | SET cat_id = :targetCatId |
||
| 116 | WHERE iid = :fileId |
||
| 117 | AND c_id = :cid |
||
| 118 | AND uploader_id = :uid |
||
| 119 | SQL; |
||
| 120 | |||
| 121 | return $conn->executeStatement($sql, [ |
||
| 122 | 'targetCatId' => $targetCatId, |
||
| 123 | 'fileId' => $fileId, |
||
| 124 | 'cid' => $cid, |
||
| 125 | 'uid' => $uid, |
||
| 126 | ]); |
||
| 127 | } |
||
| 128 | |||
| 129 | $conn->beginTransaction(); |
||
| 130 | try { |
||
| 131 | // Check recipient visibility exists |
||
| 132 | $check = <<<SQL |
||
| 133 | SELECT 1 |
||
| 134 | FROM c_dropbox_person |
||
| 135 | WHERE c_id = :cid |
||
| 136 | AND file_id = :fileId |
||
| 137 | AND user_id = :uid |
||
| 138 | LIMIT 1 |
||
| 139 | SQL; |
||
| 140 | $exists = (bool) $conn->fetchOne($check, [ |
||
| 141 | 'cid' => $cid, |
||
| 142 | 'fileId' => $fileId, |
||
| 143 | 'uid' => $uid, |
||
| 144 | ]); |
||
| 145 | |||
| 146 | if (!$exists) { |
||
| 147 | // Not a recipient; nothing to move. |
||
| 148 | $conn->commit(); |
||
| 149 | return 0; |
||
| 150 | } |
||
| 151 | |||
| 152 | // Update the file's category. |
||
| 153 | $upd = <<<SQL |
||
| 154 | UPDATE c_dropbox_file |
||
| 155 | SET cat_id = :targetCatId |
||
| 156 | WHERE iid = :fileId |
||
| 157 | AND c_id = :cid |
||
| 158 | SQL; |
||
| 159 | $affected = $conn->executeStatement($upd, [ |
||
| 160 | 'targetCatId' => $targetCatId, |
||
| 161 | 'fileId' => $fileId, |
||
| 162 | 'cid' => $cid, |
||
| 163 | ]); |
||
| 164 | |||
| 165 | $conn->commit(); |
||
| 166 | return $affected; |
||
| 167 | } catch (\Throwable $e) { |
||
| 168 | $conn->rollBack(); |
||
| 169 | throw $e; |
||
| 170 | } |
||
| 228 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)or! empty(...)instead.