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:
1 | <?php |
||
91 | class BuildTestEnvironment_Command extends BaseCommand implements CliSignalHandler |
||
92 | { |
||
93 | // we need to track this for handling CTRL-C |
||
94 | protected $st; |
||
95 | |||
96 | // we track this for convenience |
||
97 | protected $output; |
||
98 | |||
99 | // our list of players to execute |
||
100 | protected $playerList; |
||
101 | |||
102 | // our injected data / services |
||
103 | // needed for when user presses CTRL+C |
||
104 | protected $injectables; |
||
105 | |||
106 | View Code Duplication | public function __construct($injectables) |
|
134 | |||
135 | /** |
||
136 | * |
||
137 | * @param CliEngine $engine |
||
138 | * @param array $params |
||
139 | * @param Injectables|null $injectables |
||
140 | * @return integer |
||
141 | */ |
||
142 | View Code Duplication | public function processCommand(CliEngine $engine, $params = array(), $injectables = null) |
|
168 | |||
169 | public function processInsideLegacyHandler(CliEngine $engine, $params = array(), $injectables = null) |
||
170 | { |
||
171 | // process the common functionality |
||
172 | $this->initFeaturesBeforeModulesAvailable($engine); |
||
173 | |||
174 | // now it is safe to create our shorthand |
||
175 | $runtimeConfig = $injectables->getRuntimeConfig(); |
||
176 | $runtimeConfigManager = $injectables->getRuntimeConfigManager(); |
||
177 | $output = $injectables->output; |
||
178 | |||
179 | // save the output for use in other methods |
||
180 | $this->output = $output; |
||
181 | |||
182 | // at this point, all of the services / data held in $injectables |
||
183 | // has been initialised and is ready for use |
||
184 | // |
||
185 | // what's left is the stuff that needs initialising in phases |
||
186 | // or $st |
||
187 | |||
188 | // create a new StoryTeller object |
||
189 | $st = new StoryTeller($injectables); |
||
190 | |||
191 | // remember our $st object, as we'll need it for our |
||
192 | // shutdown function |
||
193 | $this->st = $st; |
||
194 | |||
195 | // now that we have $st, we can initialise any feature that |
||
196 | // wants to use our modules |
||
197 | $this->initFeaturesAfterModulesAvailable($st, $engine, $injectables); |
||
198 | |||
199 | // install signal handling, now that $this->st is defined |
||
200 | // |
||
201 | // we wouldn't want signal handling called out of order :) |
||
202 | $this->initSignalHandling($injectables); |
||
203 | |||
204 | // build our list of players to run |
||
205 | $this->initPlayerList($injectables); |
||
206 | |||
207 | // let's keep score :) |
||
208 | $startTime = microtime(true); |
||
209 | |||
210 | // and we're ready to tell the world that we're here |
||
211 | $output->startStoryplayer( |
||
212 | $engine->getAppVersion(), |
||
213 | $engine->getAppUrl(), |
||
214 | $engine->getAppCopyright(), |
||
215 | $engine->getAppLicense() |
||
216 | ); |
||
217 | |||
218 | // $this->playerList contains one or more things to play |
||
219 | // |
||
220 | // let's play each of them in order |
||
221 | foreach ($this->playerList as $player) |
||
222 | { |
||
223 | // execute each player in turn |
||
224 | // |
||
225 | // they may also have their own list of nested players |
||
226 | $player->play($st, $injectables); |
||
227 | |||
228 | // make sure the test device has been stopped |
||
229 | // (it may have been persisted by the story) |
||
230 | // |
||
231 | // we do not allow the test device to persist between |
||
232 | // top-level players |
||
233 | $st->stopDevice(); |
||
234 | } |
||
235 | |||
236 | // write out any changed runtime config to disk |
||
237 | $runtimeConfigManager->saveRuntimeConfig($runtimeConfig, $output); |
||
238 | |||
239 | // how long did that take? |
||
240 | $duration = microtime(true) - $startTime; |
||
241 | |||
242 | // tell the output plugins that we're all done |
||
243 | $output->endStoryplayer($duration); |
||
244 | |||
245 | // all done |
||
246 | return 0; |
||
247 | } |
||
248 | |||
249 | // ================================================================== |
||
250 | // |
||
251 | // the individual initX() methods |
||
252 | // |
||
253 | // these are processed *after* the objects defined in the |
||
254 | // CommonFunctionalitySupport trait have been initialised |
||
255 | // |
||
256 | // ------------------------------------------------------------------ |
||
257 | |||
258 | /** |
||
259 | * |
||
260 | * @param Injectables $injectables |
||
261 | * @return void |
||
262 | */ |
||
263 | View Code Duplication | protected function initSignalHandling(Injectables $injectables) |
|
272 | |||
273 | /** |
||
274 | * |
||
275 | * @param Injectables $injectables |
||
276 | * @return void |
||
277 | */ |
||
278 | protected function initPlayerList(Injectables $injectables) |
||
283 | |||
284 | // ================================================================== |
||
285 | // |
||
286 | // SIGNAL handling |
||
287 | // |
||
288 | // ------------------------------------------------------------------ |
||
289 | |||
290 | /** |
||
291 | * |
||
292 | * @param integer $signo |
||
293 | * @return void |
||
294 | */ |
||
295 | View Code Duplication | public function sigtermHandler($signo) |
|
324 | } |
||
325 |
An exit expression should only be used in rare cases. For example, if you write a short command line script.
In most cases however, using an
exit
expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage.