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 Manager 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 Manager, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace Comodojo\Extender\Task; |
||
| 33 | class Manager { |
||
| 34 | |||
| 35 | use ConfigurationTrait; |
||
| 36 | use LoggerTrait; |
||
| 37 | use EventsTrait; |
||
| 38 | use TasksTableTrait; |
||
| 39 | |||
| 40 | /** |
||
| 41 | * @var int |
||
| 42 | */ |
||
| 43 | protected $lagger_timeout; |
||
| 44 | |||
| 45 | /** |
||
| 46 | * @var bool |
||
| 47 | */ |
||
| 48 | protected $multithread; |
||
| 49 | |||
| 50 | /** |
||
| 51 | * @var int |
||
| 52 | */ |
||
| 53 | protected $max_runtime; |
||
| 54 | |||
| 55 | /** |
||
| 56 | * @var int |
||
| 57 | */ |
||
| 58 | protected $max_childs; |
||
| 59 | |||
| 60 | /** |
||
| 61 | * @var Ipc |
||
| 62 | */ |
||
| 63 | protected $ipc; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * @var Locker |
||
| 67 | */ |
||
| 68 | protected $locker; |
||
| 69 | |||
| 70 | /** |
||
| 71 | * @var Tracker |
||
| 72 | */ |
||
| 73 | protected $tracker; |
||
| 74 | |||
| 75 | /** |
||
| 76 | * Class constructor |
||
| 77 | * |
||
| 78 | * @param string $manager_name |
||
|
|
|||
| 79 | * @param Configuration $configuration |
||
| 80 | * @param LoggerInterface $logger |
||
| 81 | * @param TasksTable $tasks |
||
| 82 | * @param EventsManager $events |
||
| 83 | * @param EntityManager $em |
||
| 84 | */ |
||
| 85 | public function __construct( |
||
| 86 | Locker $locker, |
||
| 87 | Configuration $configuration, |
||
| 88 | LoggerInterface $logger, |
||
| 89 | TasksTable $tasks, |
||
| 90 | EventsManager $events |
||
| 91 | ) { |
||
| 92 | |||
| 93 | $this->setConfiguration($configuration); |
||
| 94 | $this->setLogger($logger); |
||
| 95 | $this->setTasksTable($tasks); |
||
| 96 | $this->setEvents($events); |
||
| 97 | |||
| 98 | $this->locker = $locker; |
||
| 99 | $this->tracker = new Tracker($configuration, $logger); |
||
| 100 | $this->ipc = new Ipc($configuration); |
||
| 101 | |||
| 102 | // retrieve parameters |
||
| 103 | $this->lagger_timeout = ExtenderCommonValidations::laggerTimeout($this->configuration->get('child-lagger-timeout')); |
||
| 104 | $this->multithread = ExtenderCommonValidations::multithread($this->configuration->get('multithread')); |
||
| 105 | $this->max_runtime = ExtenderCommonValidations::maxChildRuntime($this->configuration->get('child-max-runtime')); |
||
| 106 | $this->max_childs = ExtenderCommonValidations::forkLimit($this->configuration->get('fork-limit')); |
||
| 107 | |||
| 108 | // $logger->debug("Tasks Manager online", array( |
||
| 109 | // 'lagger_timeout' => $this->lagger_timeout, |
||
| 110 | // 'multithread' => $this->multithread, |
||
| 111 | // 'max_runtime' => $this->max_runtime, |
||
| 112 | // 'max_childs' => $this->max_childs, |
||
| 113 | // 'tasks_count' => count($this->table) |
||
| 114 | // )); |
||
| 115 | |||
| 116 | set_error_handler([$this, 'customErrorHandler']); |
||
| 117 | |||
| 118 | } |
||
| 119 | |||
| 120 | public function __destruct() { |
||
| 121 | |||
| 122 | restore_error_handler(); |
||
| 123 | |||
| 124 | } |
||
| 125 | |||
| 126 | public function add(Request $request) { |
||
| 127 | |||
| 128 | $this->tracker->setQueued($request); |
||
| 129 | |||
| 130 | return $this; |
||
| 131 | |||
| 132 | } |
||
| 133 | |||
| 134 | public function addBulk(array $requests) { |
||
| 135 | |||
| 136 | foreach ($requests as $id => $request) { |
||
| 137 | |||
| 138 | if ($request instanceof \Comodojo\Extender\Task\Request) { |
||
| 139 | $this->add($request); |
||
| 140 | } else { |
||
| 141 | $this->logger->error("Skipping invalid request with local id $id: class mismatch"); |
||
| 142 | } |
||
| 143 | |||
| 144 | } |
||
| 145 | |||
| 146 | return $this; |
||
| 147 | |||
| 148 | } |
||
| 149 | |||
| 150 | public function run() { |
||
| 151 | |||
| 152 | $this->updateTrackerSetQueued(); |
||
| 153 | |||
| 154 | while ( $this->tracker->countQueued() > 0 ) { |
||
| 155 | |||
| 156 | // Start to cycle queued tasks |
||
| 157 | $this->cycle(); |
||
| 158 | |||
| 159 | } |
||
| 160 | |||
| 161 | $this->ipc->free(); |
||
| 162 | |||
| 163 | return $this->tracker->getCompleted(); |
||
| 164 | |||
| 165 | } |
||
| 166 | |||
| 167 | public function customErrorHandler($errno, $errstr, $errfile, $errline) { |
||
| 168 | |||
| 169 | $this->getLogger()->error("Unhandled error ($errno): $errstr [in $errfile line $errline]"); |
||
| 170 | |||
| 171 | return true; |
||
| 172 | |||
| 173 | } |
||
| 174 | |||
| 175 | protected function cycle() { |
||
| 221 | |||
| 222 | protected function runSingleThread($uid, Request $request) { |
||
| 223 | |||
| 224 | $pid = ProcessTools::getPid(); |
||
| 225 | |||
| 226 | $this->updateTrackerSetRunning($uid, $pid); |
||
| 227 | |||
| 244 | |||
| 245 | private function forker(Request $request) { |
||
| 302 | |||
| 303 | private function catcher_loop() { |
||
| 312 | |||
| 313 | /** |
||
| 314 | * Catch results from completed jobs |
||
| 315 | * |
||
| 316 | */ |
||
| 317 | private function catcher() { |
||
| 390 | |||
| 391 | private function evalChain(Request $request, Result $result) { |
||
| 415 | |||
| 416 | private function generateSyntheticResult($uid, $message, $jid = null, $success = true) { |
||
| 431 | |||
| 432 | private function updateTrackerSetQueued() { |
||
| 439 | |||
| 440 | private function updateTrackerSetRunning($uid, $pid) { |
||
| 449 | |||
| 450 | private function updateTrackerSetCompleted($uid, $result) { |
||
| 468 | |||
| 469 | private function updateTrackerSetAborted($uid, $result) { |
||
| 478 | |||
| 479 | // private function updateLocker() { |
||
| 480 | // |
||
| 481 | // $this->locker->lock([ |
||
| 482 | // 'QUEUED' => $this->tracker->countQueued(), |
||
| 483 | // 'RUNNING' => $this->tracker->countRunning(), |
||
| 484 | // 'COMPLETED' => $this->tracker->countCompleted(), |
||
| 485 | // 'SUCCEEDED' => $this->tracker->countSucceeded(), |
||
| 486 | // 'FAILED' => $this->tracker->countFailed() |
||
| 487 | // ]); |
||
| 488 | // |
||
| 489 | // } |
||
| 490 | |||
| 491 | } |
||
| 492 |
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function.
Consider the following example. The parameter
$italyis not defined by the methodfinale(...).The most likely cause is that the parameter was removed, but the annotation was not.