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 ParallelExecutor 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 ParallelExecutor, and based on these observations, apply Extract Interface, too.
1 | <?php declare(strict_types=1); |
||
31 | class ParallelExecutor |
||
32 | { |
||
33 | private $input; |
||
34 | private $output; |
||
35 | private $messenger; |
||
36 | private $client; |
||
37 | private $config; |
||
38 | |||
39 | 7 | public function __construct( |
|
53 | |||
54 | /** |
||
55 | * @param Host[] $hosts |
||
56 | */ |
||
57 | 7 | private function connect(array $hosts) |
|
84 | |||
85 | /** |
||
86 | * @param Task[] $tasks |
||
87 | * @param Host[] $hosts |
||
88 | */ |
||
89 | 7 | public function run(array $tasks, array $hosts, $plan = null): int |
|
90 | { |
||
91 | 7 | $plan || $this->connect($hosts); |
|
92 | |||
93 | 7 | $localhost = new Localhost(); |
|
|
|||
94 | 7 | $globalLimit = (int)$this->input->getOption('limit') ?: count($hosts); |
|
95 | |||
96 | 7 | foreach ($tasks as $task) { |
|
97 | 7 | $plan || $this->messenger->startTask($task); |
|
98 | |||
99 | 7 | $limit = min($globalLimit, $task->getLimit() ?? $globalLimit); |
|
100 | 7 | $selector = $task->getSelector() ?? Selector::parse('all'); |
|
101 | |||
102 | 7 | if ($limit === 1 || count($hosts) === 1) { |
|
103 | 6 | foreach ($hosts as $host) { |
|
104 | 6 | if (!Selector::apply($selector, $host)) { |
|
105 | if ($plan) { |
||
106 | $plan->commit([], $task); |
||
107 | } |
||
108 | continue; |
||
109 | } |
||
110 | |||
111 | 6 | if ($plan) { |
|
112 | $plan->commit([$host], $task); |
||
113 | continue; |
||
114 | } |
||
115 | |||
116 | try { |
||
117 | 6 | $host->getConfig()->load(); |
|
118 | 6 | Exception::setTaskSourceLocation($task->getSourceLocation()); |
|
119 | |||
120 | 6 | $task->run(new Context($host, $this->input, $this->output)); |
|
121 | |||
122 | 6 | $host->getConfig()->save(); |
|
123 | 1 | } catch (GracefulShutdownException $exception) { |
|
124 | $this->messenger->renderException($exception, $host); |
||
125 | return GracefulShutdownException::EXIT_CODE; |
||
126 | 1 | } catch (\Throwable $exception) { |
|
127 | 1 | $this->messenger->renderException($exception, $host); |
|
128 | 1 | return 1; |
|
129 | } |
||
130 | } |
||
131 | } else { |
||
132 | 1 | foreach (array_chunk($hosts, $limit) as $chunk) { |
|
133 | 1 | $exitCode = $this->runTask($chunk, $task, $plan); |
|
134 | 1 | if ($exitCode !== 0) { |
|
135 | return $exitCode; |
||
136 | } |
||
137 | } |
||
138 | } |
||
139 | |||
140 | 7 | $this->messenger->endTask($task); |
|
141 | } |
||
142 | |||
143 | 7 | return 0; |
|
144 | } |
||
145 | |||
146 | 1 | private function runTask(array $hosts, Task $task, Planner $plan = null): int |
|
185 | |||
186 | 1 | protected function getProcess(Host $host, Task $task): Process |
|
199 | |||
200 | /** |
||
201 | * @param Process[] $processes |
||
202 | */ |
||
203 | 1 | protected function startProcesses(array $processes) |
|
209 | |||
210 | /** |
||
211 | * @param Process[] $processes |
||
212 | */ |
||
213 | 1 | protected function areRunning(array $processes): bool |
|
223 | |||
224 | /** |
||
225 | * @param Process[] $processes |
||
226 | */ |
||
227 | 1 | protected function gatherOutput(array $processes, callable $callback) |
|
241 | |||
242 | /** |
||
243 | * Gather the cumulative exit code for the processes. |
||
244 | */ |
||
245 | 1 | protected function cumulativeExitCode(array $processes): int |
|
255 | } |
||
256 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVar
assignment in line 1 and the$higher
assignment in line 2 are dead. The first because$myVar
is never used and the second because$higher
is always overwritten for every possible time line.