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 LogfileCommand 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 LogfileCommand, and based on these observations, apply Extract Interface, too.
| 1 | <?php  | 
            ||
| 70 | class LogfileCommand extends Command  | 
            ||
| 71 | { | 
            ||
| 72 | /**  | 
            ||
| 73 | * @var array  | 
            ||
| 74 | */  | 
            ||
| 75 | private $undefinedClients = array();  | 
            ||
| 76 | |||
| 77 | private $uas = array();  | 
            ||
| 78 | private $uasWithType = array();  | 
            ||
| 79 | |||
| 80 | private $countOk = 0;  | 
            ||
| 81 | private $countNok = 0;  | 
            ||
| 82 | private $totalCount = 0;  | 
            ||
| 83 | |||
| 84 | /**  | 
            ||
| 85 | * @var BrowscapCacheInterface  | 
            ||
| 86 | */  | 
            ||
| 87 | private $cache = null;  | 
            ||
| 88 | |||
| 89 | /**  | 
            ||
| 90 | * @var string  | 
            ||
| 91 | */  | 
            ||
| 92 | private $defaultCacheFolder;  | 
            ||
| 93 | |||
| 94 | /**  | 
            ||
| 95 | * @param string $defaultCacheFolder  | 
            ||
| 96 | */  | 
            ||
| 97 | 1 | public function __construct($defaultCacheFolder)  | 
            |
| 98 |     { | 
            ||
| 99 | 1 | $this->defaultCacheFolder = $defaultCacheFolder;  | 
            |
| 100 | |||
| 101 | 1 | parent::__construct();  | 
            |
| 102 | 1 | }  | 
            |
| 103 | |||
| 104 | /**  | 
            ||
| 105 | * @param \BrowscapPHP\Cache\BrowscapCacheInterface $cache  | 
            ||
| 106 | *  | 
            ||
| 107 | * @return $this  | 
            ||
| 108 | */  | 
            ||
| 109 | 1 | public function setCache(BrowscapCacheInterface $cache)  | 
            |
| 115 | |||
| 116 | /**  | 
            ||
| 117 | * Configures the current command.  | 
            ||
| 118 | */  | 
            ||
| 119 | 1 | protected function configure()  | 
            |
| 171 | |||
| 172 | /**  | 
            ||
| 173 | * @param InputInterface $input  | 
            ||
| 174 | * @param OutputInterface $output  | 
            ||
| 175 | *  | 
            ||
| 176 | * @throws \UnexpectedValueException  | 
            ||
| 177 | * @throws \BrowscapPHP\Exception\InvalidArgumentException  | 
            ||
| 178 | * @return int|null|void  | 
            ||
| 179 | */  | 
            ||
| 180 | protected function execute(InputInterface $input, OutputInterface $output)  | 
            ||
| 181 |     { | 
            ||
| 182 |         if (!$input->getOption('log-file') && !$input->getOption('log-dir')) { | 
            ||
| 183 |             throw InvalidArgumentException::oneOfCommandArguments('log-file', 'log-dir'); | 
            ||
| 184 | }  | 
            ||
| 185 | |||
| 186 | $loggerHelper = new LoggerHelper();  | 
            ||
| 187 |         $logger       = $loggerHelper->create($input->getOption('debug')); | 
            ||
| 188 | |||
| 189 | $browscap = new Browscap();  | 
            ||
| 190 | $loader = new IniLoader();  | 
            ||
| 
                                                                                                    
                        
                         | 
                |||
| 191 | $collection = ReaderFactory::factory();  | 
            ||
| 192 | $fs = new Filesystem();  | 
            ||
| 193 | |||
| 194 | $browscap  | 
            ||
| 195 | ->setLogger($logger)  | 
            ||
| 196 | ->setCache($this->getCache($input))  | 
            ||
| 197 | ;  | 
            ||
| 198 | |||
| 199 | /** @var $file \Symfony\Component\Finder\SplFileInfo */  | 
            ||
| 200 |         foreach ($this->getFiles($input) as $file) { | 
            ||
| 201 | $this->uas = array();  | 
            ||
| 202 | $path = $this->getPath($file);  | 
            ||
| 203 | |||
| 204 | $this->countOk = 0;  | 
            ||
| 205 | $this->countNok = 0;  | 
            ||
| 206 | |||
| 207 |             $logger->info('Analyzing file "'.$file->getPathname().'"'); | 
            ||
| 208 | |||
| 209 | $lines = file($path);  | 
            ||
| 210 | |||
| 211 |             if (empty($lines)) { | 
            ||
| 212 |                 $logger->info('Skipping empty file "'.$file->getPathname().'"'); | 
            ||
| 213 | continue;  | 
            ||
| 214 | }  | 
            ||
| 215 | |||
| 216 | $this->totalCount = count($lines);  | 
            ||
| 217 | |||
| 218 |             foreach ($lines as $line) { | 
            ||
| 219 | $this->handleLine(  | 
            ||
| 220 | $output,  | 
            ||
| 221 | $collection,  | 
            ||
| 222 | $browscap,  | 
            ||
| 223 | $line  | 
            ||
| 224 | );  | 
            ||
| 225 | }  | 
            ||
| 226 | |||
| 227 | $this->outputProgress($output, '', true);  | 
            ||
| 228 | |||
| 229 | arsort($this->uas, SORT_NUMERIC);  | 
            ||
| 230 | |||
| 231 |             try { | 
            ||
| 232 | $fs->dumpFile(  | 
            ||
| 233 |                     $input->getArgument('output').'/output.txt', | 
            ||
| 234 | implode(PHP_EOL, array_unique($this->undefinedClients))  | 
            ||
| 235 | );  | 
            ||
| 236 |             } catch (IOException $e) { | 
            ||
| 237 | // do nothing  | 
            ||
| 238 | }  | 
            ||
| 239 | |||
| 240 |             try { | 
            ||
| 241 | $fs->dumpFile(  | 
            ||
| 242 |                     $input->getArgument('output').'/output-with-amount.txt', | 
            ||
| 243 | $this->createAmountContent()  | 
            ||
| 244 | );  | 
            ||
| 245 |             } catch (IOException $e) { | 
            ||
| 246 | // do nothing  | 
            ||
| 247 | }  | 
            ||
| 248 | |||
| 249 |             try { | 
            ||
| 250 | $fs->dumpFile(  | 
            ||
| 251 |                     $input->getArgument('output').'/output-with-amount-and-type.txt', | 
            ||
| 252 | $this->createAmountTypeContent()  | 
            ||
| 253 | );  | 
            ||
| 254 |             } catch (IOException $e) { | 
            ||
| 255 | // do nothing  | 
            ||
| 256 | }  | 
            ||
| 257 | }  | 
            ||
| 258 | |||
| 259 |         $outputFile = $input->getArgument('output').'/output.txt'; | 
            ||
| 260 | |||
| 261 |         try { | 
            ||
| 262 | $fs->dumpFile(  | 
            ||
| 263 | $outputFile,  | 
            ||
| 264 | implode(PHP_EOL, array_unique($this->undefinedClients))  | 
            ||
| 265 | );  | 
            ||
| 266 |         } catch (IOException $e) { | 
            ||
| 267 |             throw new \UnexpectedValueException('writing to file "'.$outputFile.'" failed', 0, $e); | 
            ||
| 268 | }  | 
            ||
| 269 | |||
| 270 |         try { | 
            ||
| 271 | $fs->dumpFile(  | 
            ||
| 272 |                 $input->getArgument('output').'/output-with-amount.txt', | 
            ||
| 273 | $this->createAmountContent()  | 
            ||
| 274 | );  | 
            ||
| 275 |         } catch (IOException $e) { | 
            ||
| 276 | // do nothing  | 
            ||
| 277 | }  | 
            ||
| 278 | |||
| 279 |         try { | 
            ||
| 280 | $fs->dumpFile(  | 
            ||
| 281 |                 $input->getArgument('output').'/output-with-amount-and-type.txt', | 
            ||
| 282 | $this->createAmountTypeContent()  | 
            ||
| 283 | );  | 
            ||
| 284 |         } catch (IOException $e) { | 
            ||
| 285 | // do nothing  | 
            ||
| 286 | }  | 
            ||
| 287 | }  | 
            ||
| 288 | |||
| 289 | private function createAmountContent()  | 
            ||
| 290 |     { | 
            ||
| 291 | $counts = array();  | 
            ||
| 292 | |||
| 293 |         foreach ($this->uasWithType as $uas) { | 
            ||
| 294 |             foreach ($uas as $userAgentString => $count) { | 
            ||
| 295 |                 if (isset($counts[$userAgentString])) { | 
            ||
| 296 | $counts[$userAgentString] += $count;  | 
            ||
| 297 |                 } else { | 
            ||
| 298 | $counts[$userAgentString] = $count;  | 
            ||
| 299 | }  | 
            ||
| 300 | }  | 
            ||
| 301 | }  | 
            ||
| 302 | |||
| 303 | $content = '';  | 
            ||
| 304 | |||
| 305 | arsort($counts, SORT_NUMERIC);  | 
            ||
| 306 | |||
| 307 |         foreach ($counts as $agentOfLine => $count) { | 
            ||
| 308 | $content .= "$count\t$agentOfLine\n";  | 
            ||
| 309 | }  | 
            ||
| 310 | |||
| 311 | return $content;  | 
            ||
| 312 | }  | 
            ||
| 313 | |||
| 314 | private function createAmountTypeContent()  | 
            ||
| 333 | |||
| 334 | /**  | 
            ||
| 335 | * @param \Symfony\Component\Console\Output\OutputInterface $output  | 
            ||
| 336 | * @param \BrowscapPHP\Util\Logfile\ReaderCollection $collection  | 
            ||
| 337 | * @param \BrowscapPHP\Browscap $browscap  | 
            ||
| 338 | * @param integer $line  | 
            ||
| 339 | *  | 
            ||
| 340 | * @throws UnknownBrowserException  | 
            ||
| 341 | * @throws UnknownBrowserTypeException  | 
            ||
| 342 | * @throws UnknownDeviceException  | 
            ||
| 343 | * @throws UnknownEngineException  | 
            ||
| 344 | * @throws UnknownPlatformException  | 
            ||
| 345 | * @throws \Exception  | 
            ||
| 346 | */  | 
            ||
| 347 | private function handleLine(OutputInterface $output, ReaderCollection $collection, Browscap $browscap, $line)  | 
            ||
| 348 |     { | 
            ||
| 349 | $userAgentString = '';  | 
            ||
| 350 | |||
| 351 |         try { | 
            ||
| 352 | $userAgentString = $collection->read($line);  | 
            ||
| 353 | |||
| 354 |             try { | 
            ||
| 355 | $this->getResult($browscap->getBrowser($userAgentString));  | 
            ||
| 356 |             } catch (\Exception $e) { | 
            ||
| 357 | $this->undefinedClients[] = $userAgentString;  | 
            ||
| 358 | |||
| 359 | throw $e;  | 
            ||
| 360 | }  | 
            ||
| 361 | |||
| 362 | $type = '.';  | 
            ||
| 363 | $this->countOk++;  | 
            ||
| 364 |         } catch (ReaderException $e) { | 
            ||
| 365 | $type = 'E';  | 
            ||
| 366 | $this->countNok++;  | 
            ||
| 367 |         } catch (UnknownBrowserTypeException $e) { | 
            ||
| 368 | $type = 'T';  | 
            ||
| 369 | $this->countNok++;  | 
            ||
| 370 |         } catch (UnknownBrowserException $e) { | 
            ||
| 371 | $type = 'B';  | 
            ||
| 372 | $this->countNok++;  | 
            ||
| 373 |         } catch (UnknownPlatformException $e) { | 
            ||
| 374 | $type = 'P';  | 
            ||
| 375 | $this->countNok++;  | 
            ||
| 376 |         } catch (UnknownDeviceException $e) { | 
            ||
| 377 | $type = 'D';  | 
            ||
| 378 | $this->countNok++;  | 
            ||
| 379 |         } catch (UnknownEngineException $e) { | 
            ||
| 380 | $type = 'N';  | 
            ||
| 381 | $this->countNok++;  | 
            ||
| 382 |         } catch (\Exception $e) { | 
            ||
| 383 | $type = 'U';  | 
            ||
| 384 | $this->countNok++;  | 
            ||
| 385 | }  | 
            ||
| 386 | |||
| 387 | $this->outputProgress($output, $type);  | 
            ||
| 388 | |||
| 389 | // count all useragents  | 
            ||
| 390 |         if (isset($this->uas[$userAgentString])) { | 
            ||
| 391 | $this->uas[$userAgentString]++;  | 
            ||
| 392 |         } else { | 
            ||
| 393 | $this->uas[$userAgentString] = 1;  | 
            ||
| 394 | }  | 
            ||
| 395 | |||
| 396 |         if ('.' !== $type && 'E' !== $type) { | 
            ||
| 397 | // count all undetected useragents grouped by detection error  | 
            ||
| 398 |             if (!isset($this->uasWithType[$type])) { | 
            ||
| 399 | $this->uasWithType[$type] = array();  | 
            ||
| 400 | }  | 
            ||
| 401 | |||
| 402 |             if (isset($this->uasWithType[$type][$userAgentString])) { | 
            ||
| 403 | $this->uasWithType[$type][$userAgentString]++;  | 
            ||
| 404 |             } else { | 
            ||
| 405 | $this->uasWithType[$type][$userAgentString] = 1;  | 
            ||
| 406 | }  | 
            ||
| 407 | }  | 
            ||
| 408 | }  | 
            ||
| 409 | |||
| 410 | /**  | 
            ||
| 411 | * @param \Symfony\Component\Console\Output\OutputInterface $output  | 
            ||
| 412 | * @param string $result  | 
            ||
| 413 | * @param bool $end  | 
            ||
| 414 | *  | 
            ||
| 415 | * @return int  | 
            ||
| 416 | */  | 
            ||
| 417 | private function outputProgress(OutputInterface $output, $result, $end = false)  | 
            ||
| 436 | |||
| 437 | /**  | 
            ||
| 438 | * @param \stdClass $result  | 
            ||
| 439 | *  | 
            ||
| 440 | * @return string  | 
            ||
| 441 | */  | 
            ||
| 442 | private function getResult(\stdClass $result)  | 
            ||
| 470 | |||
| 471 | /**  | 
            ||
| 472 | * @param \Symfony\Component\Console\Input\InputInterface $input  | 
            ||
| 473 | *  | 
            ||
| 474 | * @return \Symfony\Component\Finder\Finder  | 
            ||
| 475 | */  | 
            ||
| 476 | private function getFiles(InputInterface $input)  | 
            ||
| 496 | |||
| 497 | /**  | 
            ||
| 498 | * @param \Symfony\Component\Finder\SplFileInfo $file  | 
            ||
| 499 | *  | 
            ||
| 500 | * @return string  | 
            ||
| 501 | */  | 
            ||
| 502 | private function getPath(SplFileInfo $file)  | 
            ||
| 518 | |||
| 519 | /**  | 
            ||
| 520 | * @param InputInterface $input  | 
            ||
| 521 | *  | 
            ||
| 522 | * @return BrowscapCacheInterface  | 
            ||
| 523 | */  | 
            ||
| 524 | View Code Duplication | private function getCache(InputInterface $input)  | 
            |
| 533 | }  | 
            ||
| 534 | 
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVarassignment in line 1 and the$higherassignment in line 2 are dead. The first because$myVaris never used and the second because$higheris always overwritten for every possible time line.