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 OcrService 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 OcrService, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | class OcrService { |
||
32 | |||
33 | /** |
||
34 | * @var ILogger |
||
35 | */ |
||
36 | private $logger; |
||
37 | |||
38 | /** |
||
39 | * @var ITempManager |
||
40 | */ |
||
41 | private $tempM; |
||
42 | |||
43 | /** |
||
44 | * @var QueueService |
||
45 | */ |
||
46 | private $queueService; |
||
47 | |||
48 | /** |
||
49 | * @var OcrStatusMapper |
||
50 | */ |
||
51 | private $statusMapper; |
||
52 | |||
53 | /** |
||
54 | * @var FileMapper |
||
55 | */ |
||
56 | private $fileMapper; |
||
57 | |||
58 | /** |
||
59 | * @var ShareMapper |
||
60 | */ |
||
61 | private $shareMapper; |
||
62 | |||
63 | /** |
||
64 | * @var View |
||
65 | */ |
||
66 | private $view; |
||
67 | |||
68 | /** |
||
69 | * @var |
||
70 | */ |
||
71 | private $userId; |
||
72 | |||
73 | /** |
||
74 | * @var IL10N |
||
75 | */ |
||
76 | private $l10n; |
||
77 | |||
78 | /** |
||
79 | * Array of allowed mimetypes for ocr processing |
||
80 | */ |
||
81 | const ALLOWED_MIMETYPES = ['application/pdf', 'image/png', 'image/jpeg', 'image/tiff']; |
||
82 | |||
83 | /** |
||
84 | * the correct mimetype for a pdf file |
||
85 | */ |
||
86 | const MIMETYPE_PDF = 'application/pdf'; |
||
87 | |||
88 | /** |
||
89 | * the only allowed image mimetypes by tesseract |
||
90 | */ |
||
91 | const MIMETYPES_IMAGE = ['image/png', 'image/jpeg', 'image/tiff']; |
||
92 | |||
93 | /** |
||
94 | * OcrService constructor. |
||
95 | * |
||
96 | * @param ITempManager $tempManager |
||
97 | * @param QueueService $queueService |
||
98 | * @param OcrStatusMapper $mapper |
||
99 | * @param View $view |
||
100 | * @param $userId |
||
101 | * @param IL10N $l10n |
||
102 | * @param ILogger $logger |
||
103 | */ |
||
104 | 15 | public function __construct(ITempManager $tempManager, QueueService $queueService, OcrStatusMapper $mapper, FileMapper $fileMapper, ShareMapper $shareMapper, View $view, $userId, IL10N $l10n, ILogger $logger) { |
|
115 | |||
116 | /** |
||
117 | * Gets the list of all available tesseract-ocr languages. |
||
118 | * |
||
119 | * @return string[] Languages |
||
120 | */ |
||
121 | 4 | public function listLanguages() { |
|
148 | |||
149 | /** |
||
150 | * Processes and prepares the files for ocr. |
||
151 | * Sends the stuff to the client in order to ocr async. |
||
152 | * |
||
153 | * @param string[] $languages |
||
154 | * @param array $files |
||
155 | * @return string |
||
156 | */ |
||
157 | 5 | public function process($languages, $files) { |
|
203 | |||
204 | /** |
||
205 | * A function which returns the JSONResponse for all required status checks and tasks. |
||
206 | * It will check for already processed, pending and failed ocr tasks and return them as needed. |
||
207 | * |
||
208 | * @codeCoverageIgnore |
||
209 | * @return string |
||
210 | */ |
||
211 | public function status() { |
||
228 | |||
229 | /** |
||
230 | * The command ocr:complete for occ will call this function in order to set the status. |
||
231 | * the worker should call it automatically after each processing step. |
||
232 | * |
||
233 | * @param integer $statusId |
||
234 | * @param boolean $failed |
||
235 | * @param string $errorMessage |
||
236 | */ |
||
237 | 3 | public function complete($statusId, $failed, $errorMessage) { |
|
252 | |||
253 | /** |
||
254 | * The PersonalSettingsController will have the opportunity to delete ocr status objects. |
||
255 | * |
||
256 | * @param $statusId |
||
257 | * @param string $userId |
||
258 | * @return OcrStatus |
||
259 | */ |
||
260 | 2 | public function deleteStatus($statusId, $userId) { |
|
283 | |||
284 | /** |
||
285 | * Gets all status objects for a specific user in order to list them on the personal settings page. |
||
286 | * |
||
287 | * @param string $userId |
||
288 | * @return array |
||
289 | */ |
||
290 | 1 | public function getAllForPersonal($userId) { |
|
308 | |||
309 | /** |
||
310 | * Returns a not existing file name for pdf or image processing |
||
311 | * protected as of testing issues with static methods. (Actually |
||
312 | * it will be mocked partially) FIXME: Change this behaviour as soon as the buidlNotExistingFileName function is not static anymore |
||
313 | * @codeCoverageIgnore |
||
314 | * |
||
315 | * @param File $fileInfo |
||
316 | * @return string |
||
317 | */ |
||
318 | protected function buildTargetForShared(File $fileInfo) { |
||
348 | |||
349 | /** |
||
350 | * Returns a not existing file name for pdf or image processing |
||
351 | * protected as of testing issues with static methods. (Actually |
||
352 | * it will be mocked partially) FIXME: Change this behaviour as soon as the buidlNotExistingFileName function is not static anymore |
||
353 | * @codeCoverageIgnore |
||
354 | * |
||
355 | * @param File $fileInfo |
||
356 | * @return string |
||
357 | */ |
||
358 | protected function buildTarget(File $fileInfo) { |
||
389 | |||
390 | |||
391 | /** |
||
392 | * Returns the fileInfo for each file in files and checks |
||
393 | * if it has a allowed mimetype and some other conditions. |
||
394 | * |
||
395 | * @param array $files |
||
396 | * @return File[] |
||
397 | * @throws NotFoundException |
||
398 | */ |
||
399 | 3 | private function buildFileInfo($files) { |
|
419 | |||
420 | /** |
||
421 | * Checks a Mimetype for a specific given FileInfo. |
||
422 | * @param File $fileInfo |
||
423 | */ |
||
424 | 3 | private function checkMimeType(File $fileInfo) { |
|
434 | |||
435 | /** |
||
436 | * @param File $fileInfo |
||
437 | * @return boolean|null |
||
438 | */ |
||
439 | 2 | private function checkSharedWithInitiator($fileInfo) { |
|
454 | |||
455 | /** |
||
456 | * Finishes all Processed files by copying them to the right path and deleteing the temp files. |
||
457 | * Returns the number of processed files. |
||
458 | * |
||
459 | * @codeCoverageIgnore |
||
460 | * @return array |
||
461 | */ |
||
462 | private function handleProcessed() { |
||
487 | |||
488 | /** |
||
489 | * Removes ".txt" from the newName of a ocr status |
||
490 | * |
||
491 | * @param $status OcrStatus |
||
492 | * @return string |
||
493 | */ |
||
494 | 2 | private function removeFileExtension($status) { |
|
501 | |||
502 | /** |
||
503 | * Handles all failed orders of ocr processing queue and returns the status objects. |
||
504 | * |
||
505 | * @codeCoverageIgnore |
||
506 | * @return array |
||
507 | */ |
||
508 | private function handleFailed() { |
||
524 | |||
525 | /** |
||
526 | * Handle the possible thrown Exceptions from all methods of this class. |
||
527 | * |
||
528 | * @param Exception $e |
||
529 | * @throws Exception |
||
530 | * @throws NotFoundException |
||
531 | */ |
||
532 | 5 | View Code Duplication | private function handleException($e) { |
540 | } |
||
541 |
This error could be the result of:
1. Missing dependencies
PHP Analyzer uses your
composer.json
file (if available) to determine the dependencies of your project and to determine all the available classes and functions. It expects thecomposer.json
to be in the root folder of your repository.Are you sure this class is defined by one of your dependencies, or did you maybe not list a dependency in either the
require
orrequire-dev
section?2. Missing use statement
PHP does not complain about undefined classes in
ìnstanceof
checks. For example, the following PHP code will work perfectly fine:If you have not tested against this specific condition, such errors might go unnoticed.