mambax7 /
publisher
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 XoopsMediaUploader 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 XoopsMediaUploader, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 85 | class XoopsMediaUploader |
||
| 86 | { |
||
| 87 | public $mediaName; |
||
| 88 | public $mediaType; |
||
| 89 | public $mediaSize; |
||
| 90 | public $mediaTmpName; |
||
| 91 | public $mediaError; |
||
| 92 | public $uploadDir = ''; |
||
| 93 | public $allowedMimeTypes = array(); |
||
| 94 | public $maxFileSize = 0; |
||
| 95 | public $maxWidth; |
||
| 96 | public $maxHeight; |
||
| 97 | public $targetFileName; |
||
| 98 | public $prefix; |
||
| 99 | public $ext; |
||
| 100 | public $dimension; |
||
| 101 | public $errors = array(); |
||
| 102 | public $savedDestination; |
||
| 103 | public $savedFileName; |
||
| 104 | /** |
||
| 105 | * No admin check for uploads |
||
| 106 | */ |
||
| 107 | private $noAdminSizeCheck; |
||
| 108 | |||
| 109 | /** |
||
| 110 | * Constructor |
||
| 111 | * |
||
| 112 | * @param string $uploadDir |
||
| 113 | * @param int|array $allowedMimeTypes |
||
| 114 | * @param int $maxFileSize |
||
| 115 | * @param int $maxWidth |
||
| 116 | * @param int $maxHeight |
||
| 117 | */ |
||
| 118 | public function __construct($uploadDir, $allowedMimeTypes = 0, $maxFileSize, $maxWidth = 0, $maxHeight = 0) |
||
| 119 | { |
||
| 120 | if (is_array($allowedMimeTypes)) { |
||
| 121 | $this->allowedMimeTypes = $allowedMimeTypes; |
||
| 122 | } |
||
| 123 | $this->uploadDir = $uploadDir; |
||
| 124 | $this->maxFileSize = (int)$maxFileSize; |
||
| 125 | if (isset($maxWidth)) { |
||
| 126 | $this->maxWidth = (int)$maxWidth; |
||
| 127 | } |
||
| 128 | if (isset($maxHeight)) { |
||
| 129 | $this->maxHeight = (int)$maxHeight; |
||
| 130 | } |
||
| 131 | } |
||
| 132 | |||
| 133 | /** |
||
| 134 | * @param $value |
||
| 135 | */ |
||
| 136 | public function setNoAdminSizeCheck($value) |
||
| 140 | |||
| 141 | /** |
||
| 142 | * Fetch the uploaded file |
||
| 143 | * |
||
| 144 | * @param string $mediaName Name of the file field |
||
| 145 | * @param int $index Index of the file (if more than one uploaded under that name) |
||
| 146 | * |
||
| 147 | * @global $HTTP_POST_FILES |
||
| 148 | * @return bool |
||
| 149 | */ |
||
| 150 | public function fetchMedia($mediaName, $index = null) |
||
| 151 | { |
||
| 152 | global $_FILES; |
||
| 153 | if (!isset($_FILES[$mediaName])) { |
||
| 154 | $this->setErrors('You either did not choose a file to upload or the server has insufficient read/writes to upload this file.!'); |
||
| 155 | |||
| 156 | return false; |
||
| 157 | } elseif (isset($index) && is_array($_FILES[$mediaName]['name'])) { |
||
| 158 | $index = (int)$index; |
||
| 159 | $this->mediaName = $_FILES[$mediaName]['name'][$index]; |
||
| 160 | $this->mediaType = $_FILES[$mediaName]['type'][$index]; |
||
| 161 | $this->mediaSize = $_FILES[$mediaName]['size'][$index]; |
||
| 162 | $this->mediaTmpName = $_FILES[$mediaName]['tmp_name'][$index]; |
||
| 163 | $this->mediaError = !empty($_FILES[$mediaName]['error'][$index]) ? $_FILES[$mediaName]['errir'][$index] : 0; |
||
| 164 | } else { |
||
| 165 | $mediaName = @$_FILES[$mediaName]; |
||
| 166 | $this->mediaName = $mediaName['name']; |
||
| 167 | $this->mediaName = $mediaName['name']; |
||
| 168 | $this->mediaType = $mediaName['type']; |
||
| 169 | $this->mediaSize = $mediaName['size']; |
||
| 170 | $this->mediaTmpName = $mediaName['tmp_name']; |
||
| 171 | $this->mediaError = !empty($mediaName['error']) ? $mediaName['error'] : 0; |
||
| 172 | } |
||
| 173 | $this->dimension = getimagesize($this->mediaTmpName); |
||
| 174 | $this->errors = array(); |
||
| 175 | if ((int)$this->mediaSize < 0) { |
||
| 176 | $this->setErrors('Invalid File Size'); |
||
| 177 | |||
| 178 | return false; |
||
| 179 | } |
||
| 180 | if ('' === $this->mediaName) { |
||
| 181 | $this->setErrors('Filename Is Empty'); |
||
| 182 | |||
| 183 | return false; |
||
| 184 | } |
||
| 185 | if ('none' === $this->mediaTmpName) { |
||
| 186 | $this->setErrors('No file uploaded, this is a error'); |
||
| 187 | |||
| 188 | return false; |
||
| 189 | } |
||
| 190 | if (!$this->checkMaxFileSize()) { |
||
| 191 | $this->setErrors(sprintf('File Size: %u. Maximum Size Allowed: %u', $this->mediaSize, $this->maxFileSize)); |
||
| 192 | } |
||
| 193 | View Code Duplication | if (is_array($this->dimension)) { |
|
| 194 | if (!$this->checkMaxWidth($this->dimension[0])) { |
||
| 195 | $this->setErrors(sprintf('File width: %u. Maximum width allowed: %u', $this->dimension[0], $this->maxWidth)); |
||
| 196 | } |
||
| 197 | if (!$this->checkMaxHeight($this->dimension[1])) { |
||
| 198 | $this->setErrors(sprintf('File height: %u. Maximum height allowed: %u', $this->dimension[1], $this->maxHeight)); |
||
| 199 | } |
||
| 200 | } |
||
| 201 | if (count($this->errors) > 0) { |
||
| 202 | return false; |
||
| 203 | } |
||
| 204 | if (!$this->checkMimeType()) { |
||
| 205 | $this->setErrors('MIME type not allowed: ' . $this->mediaType); |
||
| 206 | } |
||
| 207 | if (!is_uploaded_file($this->mediaTmpName)) { |
||
| 208 | switch ($this->mediaError) { |
||
| 209 | case 0: // no error; possible file attack! |
||
| 210 | $this->setErrors('There was a problem with your upload. Error: 0'); |
||
| 211 | break; |
||
| 212 | case 1: // uploaded file exceeds the upload_max_filesize directive in php.ini |
||
| 213 | //if ($this->noAdminSizeCheck) |
||
| 214 | //{ |
||
| 215 | // return true; |
||
| 216 | //} |
||
| 217 | $this->setErrors('The file you are trying to upload is too big. Error: 1'); |
||
| 218 | break; |
||
| 219 | case 2: // uploaded file exceeds the MAX_FILE_SIZE directive that was specified in the html form |
||
| 220 | $this->setErrors('The file you are trying to upload is too big. Error: 2'); |
||
| 221 | break; |
||
| 222 | case 3: // uploaded file was only partially uploaded |
||
| 223 | $this->setErrors('The file you are trying upload was only partially uploaded. Error: 3'); |
||
| 224 | break; |
||
| 225 | case 4: // no file was uploaded |
||
| 226 | $this->setErrors('No file selected for upload. Error: 4'); |
||
| 227 | break; |
||
| 228 | default: // a default error, just in case! :) |
||
| 229 | $this->setErrors('No file selected for upload. Error: 5'); |
||
| 230 | break; |
||
| 231 | } |
||
| 232 | |||
| 233 | return false; |
||
| 234 | } |
||
| 235 | |||
| 236 | return true; |
||
| 237 | } |
||
| 238 | |||
| 239 | /** |
||
| 240 | * Set the target filename |
||
| 241 | * |
||
| 242 | * @param string $value |
||
| 243 | */ |
||
| 244 | public function setTargetFileName($value) |
||
| 248 | |||
| 249 | /** |
||
| 250 | * Set the prefix |
||
| 251 | * |
||
| 252 | * @param string $value |
||
| 253 | */ |
||
| 254 | public function setPrefix($value) |
||
| 258 | |||
| 259 | /** |
||
| 260 | * Get the uploaded filename |
||
| 261 | * |
||
| 262 | * @return string |
||
| 263 | */ |
||
| 264 | public function getMediaName() |
||
| 268 | |||
| 269 | /** |
||
| 270 | * Get the type of the uploaded file |
||
| 271 | * |
||
| 272 | * @return string |
||
| 273 | */ |
||
| 274 | public function getMediaType() |
||
| 278 | |||
| 279 | /** |
||
| 280 | * Get the size of the uploaded file |
||
| 281 | * |
||
| 282 | * @return int |
||
| 283 | */ |
||
| 284 | public function getMediaSize() |
||
| 288 | |||
| 289 | /** |
||
| 290 | * Get the temporary name that the uploaded file was stored under |
||
| 291 | * |
||
| 292 | * @return string |
||
| 293 | */ |
||
| 294 | public function getMediaTmpName() |
||
| 298 | |||
| 299 | /** |
||
| 300 | * Get the saved filename |
||
| 301 | * |
||
| 302 | * @return string |
||
| 303 | */ |
||
| 304 | public function getSavedFileName() |
||
| 308 | |||
| 309 | /** |
||
| 310 | * Get the destination the file is saved to |
||
| 311 | * |
||
| 312 | * @return string |
||
| 313 | */ |
||
| 314 | public function getSavedDestination() |
||
| 318 | |||
| 319 | /** |
||
| 320 | * Check the file and copy it to the destination |
||
| 321 | * |
||
| 322 | * @param int $chmod |
||
| 323 | * |
||
| 324 | * @return bool |
||
| 325 | */ |
||
| 326 | public function upload($chmod = 0644) |
||
| 358 | |||
| 359 | /** |
||
| 360 | * Copy the file to its destination |
||
| 361 | * |
||
| 362 | * @param $chmod |
||
| 363 | * |
||
| 364 | * @return bool |
||
| 365 | */ |
||
| 366 | public function copyFile($chmod) |
||
| 393 | |||
| 394 | /** |
||
| 395 | * Is the file the right size? |
||
| 396 | * |
||
| 397 | * @return bool |
||
| 398 | */ |
||
| 399 | public function checkMaxFileSize() |
||
| 406 | |||
| 407 | /** |
||
| 408 | * Is the picture the right width? |
||
| 409 | * |
||
| 410 | * @param $dimension |
||
| 411 | * |
||
| 412 | * @return bool |
||
| 413 | */ |
||
| 414 | public function checkMaxWidth($dimension) |
||
| 421 | |||
| 422 | /** |
||
| 423 | * Is the picture the right height? |
||
| 424 | * |
||
| 425 | * @param $dimension |
||
| 426 | * |
||
| 427 | * @return bool |
||
| 428 | */ |
||
| 429 | public function checkMaxHeight($dimension) |
||
| 436 | |||
| 437 | /** |
||
| 438 | * Is the file the right Mime type |
||
| 439 | * (is there a right type of mime? ;-) |
||
| 440 | * |
||
| 441 | * @return bool |
||
| 442 | */ |
||
| 443 | public function checkMimeType() |
||
| 451 | |||
| 452 | /** |
||
| 453 | * Add an error |
||
| 454 | * |
||
| 455 | * @param string $error |
||
| 456 | */ |
||
| 457 | public function setErrors($error) |
||
| 461 | |||
| 462 | /** |
||
| 463 | * Get generated errors |
||
| 464 | * |
||
| 465 | * @param bool $ashtml Format using HTML? |
||
| 466 | * |
||
| 467 | * @return array |string Array of array messages OR HTML string |
||
| 468 | */ |
||
| 469 | public function &getErrors($ashtml = true) |
||
| 485 | } |
||
| 486 |
Loading history...
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.