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 Upload_Validator 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 Upload_Validator, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
8 | class Upload_Validator |
||
9 | { |
||
10 | |||
11 | /** |
||
12 | * Contains a list of the max file sizes shared by |
||
13 | * all upload fields. This is then duplicated into the |
||
14 | * "allowedMaxFileSize" instance property on construct. |
||
15 | * |
||
16 | * @config |
||
17 | * @var array |
||
18 | */ |
||
19 | private static $default_max_file_size = array(); |
||
20 | |||
21 | /** |
||
22 | * Information about the temporary file produced |
||
23 | * by the PHP-runtime. |
||
24 | * |
||
25 | * @var array |
||
26 | */ |
||
27 | protected $tmpFile; |
||
28 | |||
29 | protected $errors = array(); |
||
30 | |||
31 | /** |
||
32 | * Restrict filesize for either all filetypes |
||
33 | * or a specific extension, with extension-name |
||
34 | * as array-key and the size-restriction in bytes as array-value. |
||
35 | * |
||
36 | * @var array |
||
37 | */ |
||
38 | public $allowedMaxFileSize = array(); |
||
39 | |||
40 | /** |
||
41 | * @var array Collection of extensions. |
||
42 | * Extension-names are treated case-insensitive. |
||
43 | * |
||
44 | * Example: |
||
45 | * <code> |
||
46 | * array("jpg","GIF") |
||
47 | * </code> |
||
48 | */ |
||
49 | public $allowedExtensions = array(); |
||
50 | |||
51 | /** |
||
52 | * Return all errors that occurred while validating |
||
53 | * the temporary file. |
||
54 | * |
||
55 | * @return array |
||
56 | */ |
||
57 | public function getErrors() |
||
61 | |||
62 | /** |
||
63 | * Clear out all errors |
||
64 | */ |
||
65 | public function clearErrors() |
||
69 | |||
70 | /** |
||
71 | * Set information about temporary file produced by PHP. |
||
72 | * @param array $tmpFile |
||
73 | */ |
||
74 | public function setTmpFile($tmpFile) |
||
78 | |||
79 | /** |
||
80 | * Get maximum file size for all or specified file extension. |
||
81 | * |
||
82 | * @param string $ext |
||
83 | * @return int Filesize in bytes |
||
84 | */ |
||
85 | public function getAllowedMaxFileSize($ext = null) |
||
116 | |||
117 | /** |
||
118 | * Set filesize maximums (in bytes or INI format). |
||
119 | * Automatically converts extensions to lowercase |
||
120 | * for easier matching. |
||
121 | * |
||
122 | * Example: |
||
123 | * <code> |
||
124 | * array('*' => 200, 'jpg' => 1000, '[doc]' => '5m') |
||
125 | * </code> |
||
126 | * |
||
127 | * @param array|int $rules |
||
128 | */ |
||
129 | public function setAllowedMaxFileSize($rules) |
||
153 | |||
154 | /** |
||
155 | * @return array |
||
156 | */ |
||
157 | public function getAllowedExtensions() |
||
161 | |||
162 | /** |
||
163 | * Limit allowed file extensions. Empty by default, allowing all extensions. |
||
164 | * To allow files without an extension, use an empty string. |
||
165 | * See {@link File::$allowed_extensions} to get a good standard set of |
||
166 | * extensions that are typically not harmful in a webserver context. |
||
167 | * See {@link setAllowedMaxFileSize()} to limit file size by extension. |
||
168 | * |
||
169 | * @param array $rules List of extensions |
||
170 | */ |
||
171 | public function setAllowedExtensions($rules) |
||
184 | |||
185 | /** |
||
186 | * Determines if the bytesize of an uploaded |
||
187 | * file is valid - can be defined on an |
||
188 | * extension-by-extension basis in {@link $allowedMaxFileSize} |
||
189 | * |
||
190 | * @return boolean |
||
191 | */ |
||
192 | public function isValidSize() |
||
205 | |||
206 | /** |
||
207 | * Determine if this file is valid but empty |
||
208 | * |
||
209 | * @return bool |
||
210 | */ |
||
211 | public function isFileEmpty() { |
||
218 | |||
219 | /** |
||
220 | * Determines if the temporary file has a valid extension |
||
221 | * An empty string in the validation map indicates files without an extension. |
||
222 | * @return boolean |
||
223 | */ |
||
224 | public function isValidExtension() |
||
236 | |||
237 | /** |
||
238 | * Run through the rules for this validator checking against |
||
239 | * the temporary file set by {@link setTmpFile()} to see if |
||
240 | * the file is deemed valid or not. |
||
241 | * |
||
242 | * @return boolean |
||
243 | */ |
||
244 | public function validate() |
||
290 | |||
291 | /** |
||
292 | * Check that a valid file was given for upload (ignores file size) |
||
293 | * |
||
294 | * @return bool |
||
295 | */ |
||
296 | public function isValidUpload() { |
||
311 | |||
312 | } |
||
313 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.