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 Jetpack_Media 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 Jetpack_Media, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 8 | class Jetpack_Media { |
||
| 9 | public static $WP_ORIGINAL_MEDIA = '_wp_original_post_media'; |
||
| 10 | public static $WP_REVISION_HISTORY = '_wp_revision_history'; |
||
| 11 | public static $REVISION_HISTORY_MAXIMUM_AMOUNT = 0; |
||
| 12 | public static $WP_ATTACHMENT_IMAGE_ALT = '_wp_attachment_image_alt'; |
||
| 13 | |||
| 14 | /** |
||
| 15 | * Generate a filename in function of the original filename of the media. |
||
| 16 | * The returned name has the `{basename}-{hash}-{random-number}.{ext}` shape. |
||
| 17 | * The hash is built according to the filename trying to avoid name collisions |
||
| 18 | * with other media files. |
||
| 19 | * |
||
| 20 | * @param number $media_id - media post ID |
||
| 21 | * @param string $new_filename - the new filename |
||
| 22 | * @return string A random filename. |
||
| 23 | */ |
||
| 24 | public static function generate_new_filename( $media_id, $new_filename ) { |
||
| 59 | |||
| 60 | /** |
||
| 61 | * File urls use the post (image item) date to generate a folder path. |
||
| 62 | * Post dates can change, so we use the original date used in the `guid` |
||
| 63 | * url so edits can remain in the same folder. In the following function |
||
| 64 | * we capture a string in the format of `YYYY/MM` from the guid. |
||
| 65 | * |
||
| 66 | * For example with a guid of |
||
| 67 | * "http://test.files.wordpress.com/2016/10/test.png" the resulting string |
||
| 68 | * would be: "2016/10" |
||
| 69 | * |
||
| 70 | * @param number $media_id |
||
| 71 | * @return string |
||
| 72 | */ |
||
| 73 | private function get_time_string_from_guid( $media_id ) { |
||
| 85 | |||
| 86 | /** |
||
| 87 | * Return an array of allowed mime_type items used to upload a media file. |
||
| 88 | * |
||
| 89 | * @return array mime_type array |
||
| 90 | */ |
||
| 91 | static function get_allowed_mime_types( $default_mime_types ) { |
||
| 104 | |||
| 105 | /** |
||
| 106 | * Checks that the mime type of the file |
||
| 107 | * is among those in a filterable list of mime types. |
||
| 108 | * |
||
| 109 | * @param string $file Path to file to get its mime type. |
||
| 110 | * @return bool |
||
| 111 | */ |
||
| 112 | View Code Duplication | protected static function is_file_supported_for_sideloading( $file ) { |
|
| 160 | |||
| 161 | /** |
||
| 162 | * Try to remove the temporal file from the given file array. |
||
| 163 | * |
||
| 164 | * @param array $file_array Array with data about the temporal file |
||
| 165 | * @return bool `true` if the file has been removed. `false` either the file doesn't exist or it couldn't be removed. |
||
| 166 | */ |
||
| 167 | private static function remove_tmp_file( $file_array ) { |
||
| 173 | |||
| 174 | /** |
||
| 175 | * Save the given temporal file considering file type, |
||
| 176 | * correct location according to the original file path, etc. |
||
| 177 | * The file type control is done through of `jetpack_supported_media_sideload_types` filter, |
||
| 178 | * which allows define to the users their own file types list. |
||
| 179 | * |
||
| 180 | * @param array $file_array file to save |
||
| 181 | * @param number $media_id |
||
| 182 | * @return array|WP_Error an array with information about the new file saved or a WP_Error is something went wrong. |
||
| 183 | */ |
||
| 184 | public static function save_temporary_file( $file_array, $media_id ) { |
||
| 227 | |||
| 228 | /** |
||
| 229 | * Return an object with an snapshot of a revision item. |
||
| 230 | * |
||
| 231 | * @param object $media_item - media post object |
||
| 232 | * @return object a revision item |
||
| 233 | */ |
||
| 234 | public static function get_snapshot( $media_item ) { |
||
| 249 | |||
| 250 | /** |
||
| 251 | * Add a new item into revision_history array. |
||
| 252 | * |
||
| 253 | * @param object $media_item - media post object |
||
| 254 | * @param file $file - file recently added |
||
| 255 | * @param bool $has_original_media - condition is the original media has been already added |
||
| 256 | * @return bool `true` if the item has been added. Otherwise `false`. |
||
| 257 | */ |
||
| 258 | public static function register_revision( $media_item, $file, $has_original_media ) { |
||
| 265 | /** |
||
| 266 | * Return the `revision_history` of the given media. |
||
| 267 | * |
||
| 268 | * @param number $media_id - media post ID |
||
| 269 | * @return array `revision_history` array |
||
| 270 | */ |
||
| 271 | public static function get_revision_history( $media_id ) { |
||
| 274 | |||
| 275 | /** |
||
| 276 | * Return the original media data |
||
| 277 | */ |
||
| 278 | public static function get_original_media( $media_id ) { |
||
| 283 | |||
| 284 | public static function delete_file( $pathname ) { |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Try to delete a file according to the dirname of |
||
| 295 | * the media attached file and the filename. |
||
| 296 | * |
||
| 297 | * @param number $media_id - media post ID |
||
| 298 | * @param string $filename - basename of the file ( name-of-file.ext ) |
||
| 299 | * @return bool `true` is the file has been removed, `false` if not. |
||
| 300 | */ |
||
| 301 | private static function delete_media_history_file( $media_id, $filename ) { |
||
| 320 | |||
| 321 | /** |
||
| 322 | * Remove specific items from the `revision history` array |
||
| 323 | * depending on the given criteria: array( |
||
| 324 | * 'from' => (int) <from>, |
||
| 325 | * 'to' => (int) <to>, |
||
| 326 | * ) |
||
| 327 | * |
||
| 328 | * Also, it removes the file defined in each item. |
||
| 329 | * |
||
| 330 | * @param number $media_id - media post ID |
||
| 331 | * @param object $criteria - criteria to remove the items |
||
| 332 | * @param array [$revision_history] - revision history array |
||
| 333 | * @return array `revision_history` array updated. |
||
| 334 | */ |
||
| 335 | public static function remove_items_from_revision_history( $media_id, $criteria = array(), $revision_history ) { |
||
| 362 | |||
| 363 | /** |
||
| 364 | * Limit the number of items of the `revision_history` array. |
||
| 365 | * When the stack is overflowing the oldest item is remove from there (FIFO). |
||
| 366 | * |
||
| 367 | * @param number $media_id - media post ID |
||
| 368 | * @param number [$limit] - maximun amount of items. 20 as default. |
||
| 369 | * @return array items removed from `revision_history` |
||
| 370 | */ |
||
| 371 | public static function limit_revision_history( $media_id, $limit = null) { |
||
| 392 | |||
| 393 | /** |
||
| 394 | * Remove the original file and clean the post metadata. |
||
| 395 | * |
||
| 396 | * @param number $media_id - media post ID |
||
| 397 | */ |
||
| 398 | public static function clean_original_media( $media_id ) { |
||
| 408 | |||
| 409 | /** |
||
| 410 | * Clean `revision_history` of the given $media_id. it means: |
||
| 411 | * - remove all media files tied to the `revision_history` items. |
||
| 412 | * - clean `revision_history` meta data. |
||
| 413 | * - remove and clean the `original_media` |
||
| 414 | * |
||
| 415 | * @param number $media_id - media post ID |
||
| 416 | * @return array results of removing these files |
||
| 417 | */ |
||
| 418 | public static function clean_revision_history( $media_id ) { |
||
| 437 | |||
| 438 | /** |
||
| 439 | * Edit media item process: |
||
| 440 | * |
||
| 441 | * - update attachment file |
||
| 442 | * - preserve original media file |
||
| 443 | * - trace revision history |
||
| 444 | * |
||
| 445 | * @param number $media_id - media post ID |
||
| 446 | * @param array $file_array - temporal file |
||
| 447 | * @return {Post|WP_Error} Updated media item or a WP_Error is something went wrong. |
||
| 448 | */ |
||
| 449 | public static function edit_media_file( $media_id, $file_array ) { |
||
| 496 | } |
||
| 497 | |||
| 505 |
If you define a variable conditionally, it can happen that it is not defined for all execution paths.
Let’s take a look at an example:
In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.
Available Fixes
Check for existence of the variable explicitly:
Define a default value for the variable:
Add a value for the missing path: