wikimedia /
mediawiki
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 TransformationalImageHandler 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 TransformationalImageHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 36 | abstract class TransformationalImageHandler extends ImageHandler { |
||
| 37 | /** |
||
| 38 | * @param File $image |
||
| 39 | * @param array $params Transform parameters. Entries with the keys 'width' |
||
| 40 | * and 'height' are the respective screen width and height, while the keys |
||
| 41 | * 'physicalWidth' and 'physicalHeight' indicate the thumbnail dimensions. |
||
| 42 | * @return bool |
||
| 43 | */ |
||
| 44 | function normaliseParams( $image, &$params ) { |
||
| 67 | |||
| 68 | /** |
||
| 69 | * Extracts the width/height if the image will be scaled before rotating |
||
| 70 | * |
||
| 71 | * This will match the physical size/aspect ratio of the original image |
||
| 72 | * prior to application of the rotation -- so for a portrait image that's |
||
| 73 | * stored as raw landscape with 90-degress rotation, the resulting size |
||
| 74 | * will be wider than it is tall. |
||
| 75 | * |
||
| 76 | * @param array $params Parameters as returned by normaliseParams |
||
| 77 | * @param int $rotation The rotation angle that will be applied |
||
| 78 | * @return array ($width, $height) array |
||
| 79 | */ |
||
| 80 | public function extractPreRotationDimensions( $params, $rotation ) { |
||
| 92 | |||
| 93 | /** |
||
| 94 | * Create a thumbnail. |
||
| 95 | * |
||
| 96 | * This sets up various parameters, and then calls a helper method |
||
| 97 | * based on $this->getScalerType in order to scale the image. |
||
| 98 | * |
||
| 99 | * @param File $image |
||
| 100 | * @param string $dstPath |
||
| 101 | * @param string $dstUrl |
||
| 102 | * @param array $params |
||
| 103 | * @param int $flags |
||
| 104 | * @return MediaTransformError|ThumbnailImage|TransformParameterError |
||
| 105 | */ |
||
| 106 | function doTransform( $image, $dstPath, $dstUrl, $params, $flags = 0 ) { |
||
| 288 | |||
| 289 | /** |
||
| 290 | * Get the source file for the transform |
||
| 291 | * |
||
| 292 | * @param File $file |
||
| 293 | * @param array $params |
||
| 294 | * @return array Array with keys width, height and path. |
||
| 295 | */ |
||
| 296 | protected function getThumbnailSource( $file, $params ) { |
||
| 299 | |||
| 300 | /** |
||
| 301 | * Returns what sort of scaler type should be used. |
||
| 302 | * |
||
| 303 | * Values can be one of client, im, custom, gd, imext, or an array |
||
| 304 | * of object, method-name to call that specific method. |
||
| 305 | * |
||
| 306 | * If specifying a custom scaler command with [ Obj, method ], |
||
| 307 | * the method in question should take 2 parameters, a File object, |
||
| 308 | * and a $scalerParams array with various options (See doTransform |
||
| 309 | * for what is in $scalerParams). On error it should return a |
||
| 310 | * MediaTransformError object. On success it should return false, |
||
| 311 | * and simply make sure the thumbnail file is located at |
||
| 312 | * $scalerParams['dstPath']. |
||
| 313 | * |
||
| 314 | * If there is a problem with the output path, it returns "client" |
||
| 315 | * to do client side scaling. |
||
| 316 | * |
||
| 317 | * @param string $dstPath |
||
| 318 | * @param bool $checkDstPath Check that $dstPath is valid |
||
| 319 | * @return string|Callable One of client, im, custom, gd, imext, or a Callable array. |
||
| 320 | */ |
||
| 321 | abstract protected function getScalerType( $dstPath, $checkDstPath = true ); |
||
| 322 | |||
| 323 | /** |
||
| 324 | * Get a ThumbnailImage that respresents an image that will be scaled |
||
| 325 | * client side |
||
| 326 | * |
||
| 327 | * @param File $image File associated with this thumbnail |
||
| 328 | * @param array $scalerParams Array with scaler params |
||
| 329 | * @return ThumbnailImage |
||
| 330 | * |
||
| 331 | * @todo FIXME: No rotation support |
||
| 332 | */ |
||
| 333 | protected function getClientScalingThumbnailImage( $image, $scalerParams ) { |
||
| 341 | |||
| 342 | /** |
||
| 343 | * Transform an image using ImageMagick |
||
| 344 | * |
||
| 345 | * This is a stub method. The real method is in BitmapHander. |
||
| 346 | * |
||
| 347 | * @param File $image File associated with this thumbnail |
||
| 348 | * @param array $params Array with scaler params |
||
| 349 | * |
||
| 350 | * @return MediaTransformError Error object if error occurred, false (=no error) otherwise |
||
| 351 | */ |
||
| 352 | protected function transformImageMagick( $image, $params ) { |
||
| 355 | |||
| 356 | /** |
||
| 357 | * Transform an image using the Imagick PHP extension |
||
| 358 | * |
||
| 359 | * This is a stub method. The real method is in BitmapHander. |
||
| 360 | * |
||
| 361 | * @param File $image File associated with this thumbnail |
||
| 362 | * @param array $params Array with scaler params |
||
| 363 | * |
||
| 364 | * @return MediaTransformError Error object if error occurred, false (=no error) otherwise |
||
| 365 | */ |
||
| 366 | protected function transformImageMagickExt( $image, $params ) { |
||
| 369 | |||
| 370 | /** |
||
| 371 | * Transform an image using a custom command |
||
| 372 | * |
||
| 373 | * This is a stub method. The real method is in BitmapHander. |
||
| 374 | * |
||
| 375 | * @param File $image File associated with this thumbnail |
||
| 376 | * @param array $params Array with scaler params |
||
| 377 | * |
||
| 378 | * @return MediaTransformError Error object if error occurred, false (=no error) otherwise |
||
| 379 | */ |
||
| 380 | protected function transformCustom( $image, $params ) { |
||
| 383 | |||
| 384 | /** |
||
| 385 | * Get a MediaTransformError with error 'thumbnail_error' |
||
| 386 | * |
||
| 387 | * @param array $params Parameter array as passed to the transform* functions |
||
| 388 | * @param string $errMsg Error message |
||
| 389 | * @return MediaTransformError |
||
| 390 | */ |
||
| 391 | public function getMediaTransformError( $params, $errMsg ) { |
||
| 395 | |||
| 396 | /** |
||
| 397 | * Transform an image using the built in GD library |
||
| 398 | * |
||
| 399 | * This is a stub method. The real method is in BitmapHander. |
||
| 400 | * |
||
| 401 | * @param File $image File associated with this thumbnail |
||
| 402 | * @param array $params Array with scaler params |
||
| 403 | * |
||
| 404 | * @return MediaTransformError Error object if error occurred, false (=no error) otherwise |
||
| 405 | */ |
||
| 406 | protected function transformGd( $image, $params ) { |
||
| 409 | |||
| 410 | /** |
||
| 411 | * Escape a string for ImageMagick's property input (e.g. -set -comment) |
||
| 412 | * See InterpretImageProperties() in magick/property.c |
||
| 413 | * @param string $s |
||
| 414 | * @return string |
||
| 415 | */ |
||
| 416 | function escapeMagickProperty( $s ) { |
||
| 428 | |||
| 429 | /** |
||
| 430 | * Escape a string for ImageMagick's input filenames. See ExpandFilenames() |
||
| 431 | * and GetPathComponent() in magick/utility.c. |
||
| 432 | * |
||
| 433 | * This won't work with an initial ~ or @, so input files should be prefixed |
||
| 434 | * with the directory name. |
||
| 435 | * |
||
| 436 | * Glob character unescaping is broken in ImageMagick before 6.6.1-5, but |
||
| 437 | * it's broken in a way that doesn't involve trying to convert every file |
||
| 438 | * in a directory, so we're better off escaping and waiting for the bugfix |
||
| 439 | * to filter down to users. |
||
| 440 | * |
||
| 441 | * @param string $path The file path |
||
| 442 | * @param bool|string $scene The scene specification, or false if there is none |
||
| 443 | * @throws MWException |
||
| 444 | * @return string |
||
| 445 | */ |
||
| 446 | function escapeMagickInput( $path, $scene = false ) { |
||
| 458 | |||
| 459 | /** |
||
| 460 | * Escape a string for ImageMagick's output filename. See |
||
| 461 | * InterpretImageFilename() in magick/image.c. |
||
| 462 | * @param string $path The file path |
||
| 463 | * @param bool|string $scene The scene specification, or false if there is none |
||
| 464 | * @return string |
||
| 465 | */ |
||
| 466 | function escapeMagickOutput( $path, $scene = false ) { |
||
| 471 | |||
| 472 | /** |
||
| 473 | * Armour a string against ImageMagick's GetPathComponent(). This is a |
||
| 474 | * helper function for escapeMagickInput() and escapeMagickOutput(). |
||
| 475 | * |
||
| 476 | * @param string $path The file path |
||
| 477 | * @param bool|string $scene The scene specification, or false if there is none |
||
| 478 | * @throws MWException |
||
| 479 | * @return string |
||
| 480 | */ |
||
| 481 | protected function escapeMagickPath( $path, $scene = false ) { |
||
| 505 | |||
| 506 | /** |
||
| 507 | * Retrieve the version of the installed ImageMagick |
||
| 508 | * You can use PHPs version_compare() to use this value |
||
| 509 | * Value is cached for one hour. |
||
| 510 | * @return string|bool Representing the IM version; false on error |
||
| 511 | */ |
||
| 512 | protected function getMagickVersion() { |
||
| 537 | |||
| 538 | /** |
||
| 539 | * Returns whether the current scaler supports rotation. |
||
| 540 | * |
||
| 541 | * @since 1.24 No longer static |
||
| 542 | * @return bool |
||
| 543 | */ |
||
| 544 | public function canRotate() { |
||
| 547 | |||
| 548 | /** |
||
| 549 | * Should we automatically rotate an image based on exif |
||
| 550 | * |
||
| 551 | * @since 1.24 No longer static |
||
| 552 | * @see $wgEnableAutoRotation |
||
| 553 | * @return bool Whether auto rotation is enabled |
||
| 554 | */ |
||
| 555 | public function autoRotateEnabled() { |
||
| 558 | |||
| 559 | /** |
||
| 560 | * Rotate a thumbnail. |
||
| 561 | * |
||
| 562 | * This is a stub. See BitmapHandler::rotate. |
||
| 563 | * |
||
| 564 | * @param File $file |
||
| 565 | * @param array $params Rotate parameters. |
||
| 566 | * 'rotation' clockwise rotation in degrees, allowed are multiples of 90 |
||
| 567 | * @since 1.24 Is non-static. From 1.21 it was static |
||
| 568 | * @return bool |
||
| 569 | */ |
||
| 570 | public function rotate( $file, $params ) { |
||
| 574 | |||
| 575 | /** |
||
| 576 | * Returns whether the file needs to be rendered. Returns true if the |
||
| 577 | * file requires rotation and we are able to rotate it. |
||
| 578 | * |
||
| 579 | * @param File $file |
||
| 580 | * @return bool |
||
| 581 | */ |
||
| 582 | public function mustRender( $file ) { |
||
| 585 | |||
| 586 | /** |
||
| 587 | * Check if the file is smaller than the maximum image area for thumbnailing. |
||
| 588 | * |
||
| 589 | * Runs the 'BitmapHandlerCheckImageArea' hook. |
||
| 590 | * |
||
| 591 | * @param File $file |
||
| 592 | * @param array $params |
||
| 593 | * @return bool |
||
| 594 | * @since 1.25 |
||
| 595 | */ |
||
| 596 | public function isImageAreaOkForThumbnaling( $file, &$params ) { |
||
| 624 | } |
||
| 625 |
This check looks for the bodies of
ifstatements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.These
ifbodies can be removed. If you have an empty if but statements in theelsebranch, consider inverting the condition.could be turned into
This is much more concise to read.