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 Attachments 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 Attachments, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
19 | class Attachments |
||
20 | { |
||
21 | protected $_msg = 0; |
||
22 | protected $_board = null; |
||
23 | protected $_attachmentUploadDir = false; |
||
24 | protected $_attchDir = ''; |
||
25 | protected $_currentAttachmentUploadDir; |
||
26 | protected $_canPostAttachment; |
||
27 | protected $_generalErrors = array(); |
||
28 | protected $_initialError; |
||
29 | protected $_attachments = array(); |
||
30 | protected $_attachResults = array(); |
||
31 | protected $_attachSuccess = array(); |
||
32 | protected $_response = array( |
||
33 | 'error' => true, |
||
34 | 'data' => array(), |
||
35 | 'extra' => '', |
||
36 | ); |
||
37 | protected $_subActions = array( |
||
38 | 'add', |
||
39 | 'delete', |
||
40 | ); |
||
41 | protected $_sa = false; |
||
42 | |||
43 | public function __construct() |
||
44 | { |
||
45 | global $modSettings, $context; |
||
46 | |||
47 | $this->_msg = (int) !empty($_REQUEST['msg']) ? $_REQUEST['msg'] : 0; |
||
48 | $this->_board = (int) !empty($_REQUEST['board']) ? $_REQUEST['board'] : null; |
||
49 | |||
50 | $this->_currentAttachmentUploadDir = $modSettings['currentAttachmentUploadDir']; |
||
51 | |||
52 | $this->_attachmentUploadDir = $modSettings['attachmentUploadDir']; |
||
53 | |||
54 | $this->_attchDir = $context['attach_dir'] = $this->_attachmentUploadDir[$modSettings['currentAttachmentUploadDir']]; |
||
55 | |||
56 | $this->_canPostAttachment = $context['can_post_attachment'] = !empty($modSettings['attachmentEnable']) && $modSettings['attachmentEnable'] == 1 && (allowedTo('post_attachment', $this->_board) || ($modSettings['postmod_active'] && allowedTo('post_unapproved_attachments', $this->_board))); |
||
57 | } |
||
58 | |||
59 | public function call() |
||
60 | { |
||
61 | global $smcFunc, $sourcedir; |
||
62 | |||
63 | require_once($sourcedir . '/Subs-Attachments.php'); |
||
64 | |||
65 | // Guest aren't welcome, sorry. |
||
66 | is_not_guest(); |
||
67 | |||
68 | // Need this. For reasons... |
||
69 | loadLanguage('Post'); |
||
70 | |||
71 | $this->_sa = !empty($_REQUEST['sa']) ? $smcFunc['htmlspecialchars']($smcFunc['htmltrim']($_REQUEST['sa'])) : false; |
||
72 | |||
73 | if ($this->_canPostAttachment && $this->_sa && in_array($this->_sa, $this->_subActions)) |
||
74 | $this->{$this->_sa}(); |
||
75 | |||
76 | // Just send a generic message. |
||
77 | else |
||
78 | $this->setResponse(array( |
||
79 | 'text' => $this->_sa == 'add' ? 'attach_error_title' : 'attached_file_deleted_error', |
||
80 | 'type' => 'error', |
||
81 | 'data' => false, |
||
82 | )); |
||
83 | |||
84 | // Back to the future, oh, to the browser! |
||
85 | $this->sendResponse(); |
||
86 | } |
||
87 | |||
88 | public function delete() |
||
116 | )); |
||
117 | } |
||
118 | |||
119 | public function add() |
||
120 | { |
||
121 | // You gotta be able to post attachments. |
||
122 | if (!$this->_canPostAttachment) |
||
123 | return $this->setResponse(array( |
||
124 | 'text' => 'attached_file_cannot', |
||
125 | 'type' => 'error', |
||
126 | 'data' => false, |
||
127 | )); |
||
128 | |||
129 | // Process them at once! |
||
130 | $this->processAttachments(); |
||
131 | |||
132 | // The attachments was created and moved the the right folder, time to update the DB. |
||
133 | if (!empty($_SESSION['temp_attachments'])) |
||
134 | $this->createAtttach(); |
||
135 | |||
136 | // Set the response. |
||
137 | $this->setResponse(); |
||
138 | } |
||
139 | |||
140 | /** |
||
141 | * Moves an attachment to the proper directory and set the relevant data into $_SESSION['temp_attachments'] |
||
142 | */ |
||
143 | protected function processAttachments() |
||
144 | { |
||
145 | global $context, $modSettings, $smcFunc, $user_info, $txt; |
||
146 | |||
147 | if (!isset($_FILES['attachment']['name'])) |
||
148 | $_FILES['attachment']['tmp_name'] = array(); |
||
149 | |||
150 | // If there are attachments, calculate the total size and how many. |
||
151 | $context['attachments']['total_size'] = 0; |
||
152 | $context['attachments']['quantity'] = 0; |
||
153 | |||
154 | // If this isn't a new post, check the current attachments. |
||
155 | if (isset($_REQUEST['msg'])) |
||
156 | { |
||
157 | $context['attachments']['quantity'] = count($context['current_attachments']); |
||
158 | foreach ($context['current_attachments'] as $attachment) |
||
159 | $context['attachments']['total_size'] += $attachment['size']; |
||
160 | } |
||
161 | |||
162 | // A bit of house keeping first. |
||
163 | if (!empty($_SESSION['temp_attachments']) && count($_SESSION['temp_attachments']) == 1) |
||
164 | unset($_SESSION['temp_attachments']); |
||
165 | |||
166 | // Our infamous SESSION var, we are gonna have soo much fun with it! |
||
167 | if (!isset($_SESSION['temp_attachments'])) |
||
168 | $_SESSION['temp_attachments'] = array(); |
||
169 | |||
170 | // Make sure we're uploading to the right place. |
||
171 | if (!empty($modSettings['automanage_attachments'])) |
||
172 | automanage_attachments_check_directory(); |
||
173 | |||
174 | // Is the attachments folder actually there? |
||
175 | if (!empty($context['dir_creation_error'])) |
||
176 | $this->_generalErrors[] = $context['dir_creation_error']; |
||
177 | |||
178 | // The current attach folder ha some issues... |
||
179 | elseif (!is_dir($this->_attchDir)) |
||
180 | { |
||
181 | $this->_generalErrors[] = 'attach_folder_warning'; |
||
182 | log_error(sprintf($txt['attach_folder_admin_warning'], $this->_attchDir), 'critical'); |
||
183 | } |
||
184 | |||
185 | // If this isn't a new post, check the current attachments. |
||
186 | if (empty($this->_generalErrors) && $this->_msg) |
||
187 | { |
||
188 | $context['attachments'] = array(); |
||
189 | $request = $smcFunc['db_query']('', ' |
||
190 | SELECT COUNT(*), SUM(size) |
||
191 | FROM {db_prefix}attachments |
||
192 | WHERE id_msg = {int:id_msg} |
||
193 | AND attachment_type = {int:attachment_type}', |
||
194 | array( |
||
195 | 'id_msg' => (int) $this->_msg, |
||
196 | 'attachment_type' => 0, |
||
197 | ) |
||
198 | ); |
||
199 | list ($context['attachments']['quantity'], $context['attachments']['total_size']) = $smcFunc['db_fetch_row']($request); |
||
200 | $smcFunc['db_free_result']($request); |
||
201 | } |
||
202 | |||
203 | else |
||
204 | $context['attachments'] = array( |
||
205 | 'quantity' => 0, |
||
206 | 'total_size' => 0, |
||
207 | ); |
||
208 | |||
209 | // Check for other general errors here. |
||
210 | |||
211 | // If we have an initial error, delete the files. |
||
212 | if (!empty($this->_generalErrors)) |
||
213 | { |
||
214 | // And delete the files 'cos they ain't going nowhere. |
||
215 | foreach ($_FILES['attachment']['tmp_name'] as $n => $dummy) |
||
216 | if (file_exists($_FILES['attachment']['tmp_name'][$n])) |
||
217 | unlink($_FILES['attachment']['tmp_name'][$n]); |
||
218 | |||
219 | $_FILES['attachment']['tmp_name'] = array(); |
||
220 | |||
221 | // No point in going further with this. |
||
222 | return; |
||
223 | } |
||
224 | |||
225 | // Loop through $_FILES['attachment'] array and move each file to the current attachments folder. |
||
226 | foreach ($_FILES['attachment']['tmp_name'] as $n => $dummy) |
||
227 | { |
||
228 | if ($_FILES['attachment']['name'][$n] == '') |
||
229 | continue; |
||
230 | |||
231 | // First, let's first check for PHP upload errors. |
||
232 | $errors = array(); |
||
233 | if (!empty($_FILES['attachment']['error'][$n])) |
||
234 | { |
||
235 | if ($_FILES['attachment']['error'][$n] == 2) |
||
236 | $errors[] = array('file_too_big', array($modSettings['attachmentSizeLimit'])); |
||
237 | |||
238 | else |
||
239 | log_error($_FILES['attachment']['name'][$n] . ': ' . $txt['php_upload_error_' . $_FILES['attachment']['error'][$n]]); |
||
240 | |||
241 | // Log this one, because... |
||
242 | if ($_FILES['attachment']['error'][$n] == 6) |
||
243 | log_error($_FILES['attachment']['name'][$n] . ': ' . $txt['php_upload_error_6'], 'critical'); |
||
244 | |||
245 | // Weird, no errors were cached, still fill out a generic one. |
||
246 | if (empty($errors)) |
||
247 | $errors[] = 'attach_php_error'; |
||
248 | } |
||
249 | |||
250 | // Try to move and rename the file before doing any more checks on it. |
||
251 | $attachID = 'post_tmp_' . $user_info['id'] . '_' . md5(mt_rand()); |
||
252 | $destName = $this->_attchDir . '/' . $attachID; |
||
253 | |||
254 | // No errors, YAY! |
||
255 | if (empty($errors)) |
||
256 | { |
||
257 | // The reported MIME type of the attachment might not be reliable. |
||
258 | // Fortunately, PHP 5.3+ lets us easily verify the real MIME type. |
||
259 | if (function_exists('mime_content_type')) |
||
260 | $_FILES['attachment']['type'][$n] = mime_content_type($_FILES['attachment']['tmp_name'][$n]); |
||
261 | |||
262 | $_SESSION['temp_attachments'][$attachID] = array( |
||
263 | 'name' => $smcFunc['htmlspecialchars'](basename($_FILES['attachment']['name'][$n])), |
||
264 | 'tmp_name' => $destName, |
||
265 | 'size' => $_FILES['attachment']['size'][$n], |
||
266 | 'type' => $_FILES['attachment']['type'][$n], |
||
267 | 'id_folder' => $modSettings['currentAttachmentUploadDir'], |
||
268 | 'errors' => array(), |
||
269 | ); |
||
270 | |||
271 | // Move the file to the attachments folder with a temp name for now. |
||
272 | if (@move_uploaded_file($_FILES['attachment']['tmp_name'][$n], $destName)) |
||
273 | smf_chmod($destName, 0644); |
||
274 | |||
275 | // This is madness!! |
||
276 | else |
||
277 | { |
||
278 | // File couldn't be moved. |
||
279 | $_SESSION['temp_attachments'][$attachID]['errors'][] = 'attach_timeout'; |
||
280 | if (file_exists($_FILES['attachment']['tmp_name'][$n])) |
||
281 | unlink($_FILES['attachment']['tmp_name'][$n]); |
||
282 | } |
||
283 | } |
||
284 | |||
285 | // Fill up a nice array with some data from the file and the errors encountered so far. |
||
286 | else |
||
287 | { |
||
288 | $_SESSION['temp_attachments'][$attachID] = array( |
||
289 | 'name' => $smcFunc['htmlspecialchars'](basename($_FILES['attachment']['name'][$n])), |
||
290 | 'tmp_name' => $destName, |
||
291 | 'errors' => $errors, |
||
292 | ); |
||
293 | |||
294 | if (file_exists($_FILES['attachment']['tmp_name'][$n])) |
||
295 | unlink($_FILES['attachment']['tmp_name'][$n]); |
||
296 | } |
||
297 | |||
298 | // If there's no errors to this point. We still do need to apply some additional checks before we are finished. |
||
299 | if (empty($_SESSION['temp_attachments'][$attachID]['errors'])) |
||
300 | attachmentChecks($attachID); |
||
301 | } |
||
302 | |||
303 | // Mod authors, finally a hook to hang an alternate attachment upload system upon |
||
304 | // Upload to the current attachment folder with the file name $attachID or 'post_tmp_' . $user_info['id'] . '_' . md5(mt_rand()) |
||
305 | // Populate $_SESSION['temp_attachments'][$attachID] with the following: |
||
306 | // name => The file name |
||
307 | // tmp_name => Path to the temp file ($this->_attchDir . '/' . $attachID). |
||
308 | // size => File size (required). |
||
309 | // type => MIME type (optional if not available on upload). |
||
310 | // id_folder => $modSettings['currentAttachmentUploadDir'] |
||
311 | // errors => An array of errors (use the index of the $txt variable for that error). |
||
312 | // Template changes can be done using "integrate_upload_template". |
||
313 | call_integration_hook('integrate_attachment_upload', array()); |
||
314 | } |
||
315 | |||
316 | protected function createAtttach() |
||
317 | { |
||
318 | global $txt, $user_info, $modSettings; |
||
319 | |||
320 | // Create an empty session var to keep track of all the files we attached. |
||
321 | $SESSION['already_attached'] = array(); |
||
322 | |||
323 | foreach ($_SESSION['temp_attachments'] as $attachID => $attachment) |
||
324 | { |
||
325 | $attachmentOptions = array( |
||
326 | 'post' => $this->_msg, |
||
327 | 'poster' => $user_info['id'], |
||
328 | 'name' => $attachment['name'], |
||
329 | 'tmp_name' => $attachment['tmp_name'], |
||
330 | 'size' => isset($attachment['size']) ? $attachment['size'] : 0, |
||
331 | 'mime_type' => isset($attachment['type']) ? $attachment['type'] : '', |
||
332 | 'id_folder' => isset($attachment['id_folder']) ? $attachment['id_folder'] : $modSettings['currentAttachmentUploadDir'], |
||
333 | 'approved' => !$modSettings['postmod_active'] || allowedTo('post_attachment'), |
||
334 | 'errors' => array(), |
||
335 | ); |
||
336 | |||
337 | if (empty($attachment['errors'])) |
||
338 | { |
||
339 | if (createAttachment($attachmentOptions)) |
||
340 | { |
||
341 | // Avoid JS getting confused. |
||
342 | $attachmentOptions['attachID'] = $attachmentOptions['id']; |
||
343 | unset($attachmentOptions['id']); |
||
344 | |||
345 | $_SESSION['already_attached'][$attachmentOptions['attachID']] = $attachmentOptions['attachID']; |
||
346 | |||
347 | if (!empty($attachmentOptions['thumb'])) |
||
348 | $_SESSION['already_attached'][$attachmentOptions['thumb']] = $attachmentOptions['thumb']; |
||
349 | |||
350 | if ($this->_msg) |
||
351 | assignAttachments($_SESSION['already_attached'], $this->_msg); |
||
352 | } |
||
353 | } |
||
354 | else |
||
355 | { |
||
356 | // Sort out the errors for display and delete any associated files. |
||
357 | $log_these = array('attachments_no_create', 'attachments_no_write', 'attach_timeout', 'ran_out_of_space', 'cant_access_upload_path', 'attach_0_byte_file'); |
||
358 | |||
359 | foreach ($attachment['errors'] as $error) |
||
360 | { |
||
361 | $attachmentOptions['errors'][] = vsprintf($txt['attach_warning'], $attachment['name']); |
||
362 | |||
363 | if (!is_array($error)) |
||
364 | { |
||
365 | $attachmentOptions['errors'][] = $txt[$error]; |
||
366 | if (in_array($error, $log_these)) |
||
367 | log_error($attachment['name'] . ': ' . $txt[$error], 'critical'); |
||
368 | } |
||
369 | else |
||
370 | $attachmentOptions['errors'][] = vsprintf($txt[$error[0]], $error[1]); |
||
371 | } |
||
372 | if (file_exists($attachment['tmp_name'])) |
||
373 | unlink($attachment['tmp_name']); |
||
374 | } |
||
375 | |||
376 | // You don't need to know. |
||
377 | unset($attachmentOptions['tmp_name']); |
||
378 | unset($attachmentOptions['destination']); |
||
379 | |||
380 | // Regardless of errors, pass the results. |
||
381 | $this->_attachResults[] = $attachmentOptions; |
||
382 | } |
||
383 | |||
384 | // Temp save this on the db. |
||
385 | if (!empty($_SESSION['already_attached'])) |
||
386 | $this->_attachSuccess = $_SESSION['already_attached']; |
||
387 | |||
388 | unset($_SESSION['temp_attachments']); |
||
389 | } |
||
390 | |||
391 | protected function setResponse($data = array()) |
||
425 | } |
||
426 | |||
427 | protected function sendResponse() |
||
447 | } |
||
448 | } |
||
449 | |||
450 | ?> |
This check looks for function or method calls that always return null and whose return value is used.
The method
getObject()
can return nothing but null, so it makes no sense to use the return value.The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.