How to Detect Dead PHP Code in Code Review in 7 Snippets
After few long Nette to Symfony migration series, it's time for relax.
Let's look at 7 snippets of PHP code, that happily takes your attention but is never run.
Imagine you're doing a code review of "various improvements" pull-request with 150 changed files...
Ok, not that one. Let's go only for 7 changed files. There will be 7 code snippets, each possibly with extra code that looks useful, but it doesn't do anything really.
Then click on the button to see if you were right :)
1. Duplicated Array Key
<?php
$items = [
1 => 'A',
1 => 'B',
1 => 'C'
];What is 1?
<?php
var_dump($items[1]);"A" or "C"? Or Array with all fo them?
2. Key with Key-Hole
<?php
$items = [];
$result = [];
foreach ($items as $key => $value) {
$result[] = $value;
}What value is extra here?
-foreach ($items as $key => $value) {
+foreach ($items as $value) {
$result[] = $value;
}3. Call for Nothing
<?php
class ParentButNoMethod extends ParentMethod
{
public function one()
{
parent::one();
}
public function two()
{
parent::two();
}
}
class ParentMethod
{
public function one()
{
}
}
<?php
class ParentButNoMethod extends ParentMethod
{
public function one()
{
parent::one();
}
public function two()
{
- parent::two();
}
}
class ParentMethod
{
public function one()
{
}
}4. Reborn?
<?php
class ProductController
{
public function actionDiscount(Product $product)
{
$discount = $this->getDiscount();
$productCategory = $this->categoryRepository->findCategoriesByProduct(
$product->getCategory()
);
$discount = $this->getDiscount();
return $this->render('product/discount.twig', [
'discount' => $discount,
'product' => $product,
'productCategory' => $productCategory,
]);
}
}
-$discount = $this->getDiscount();
$productCategory = $this->categoryRepository->findCategoriesByProduct(
$product->getCategory()
);
$discount = $this->getDiscount();5. Behind the Mirror
<?php
final class TimeMachine
{
public function mirrorFunction(Quiz $quiz)
{
$timeLimit = $this->resolveTimeLimitForThisTest();
if ($timeLimit >= 20) {
return false;
}
$timeLimit = $timeLimit;
if ($this->isQuizFinished($quiz)) {
$correctQuestions = 1;
$correctQuestions = $correctQuestions;
$incorrectQuestions = $correctQuestions - 3;
}
}
}
<?php
final class TimeMachine
{
public function mirrorFunction(Quiz $quiz)
{
$timeLimit = $this->resolveTimeLimitForThisTest();
if ($timeLimit >= 20) {
return false;
}
- $timeLimit = $timeLimit;
if ($this->isQuizFinished($quiz)) {
$correctQuestions = 1;
- $correctQuestions = $correctQuestions;
$incorrectQuestions = $correctQuestions - 3;
}
}
}6. Rinse & Repeat
<?php
final class WhateverMethodCall
{
public function run()
{
$directories = 1;
$anotherDirectories = 1;
$directories = 2;
$this->store($directories);
$anotherDirectories = 2;
$directories = 3;
$anotherDirectories = 3;
$directories = 4;
$directories = 5;
return $directories + $anotherDirectories;
}
public function store(int $directories)
{
}
}
<?php
final class WhateverMethodCall
{
public function run()
{
- $directories = 1;
- $anotherDirectories = 1;
$directories = 2;
$this->store($directories);
- $anotherDirectories = 2;
- $directories = 3;
$anotherDirectories = 3;
- $directories = 4;
$directories = 5;
return $directories + $anotherDirectories;
}
}7. Privates that No-One See
<?php
final class SomeController
{
private const MAX_LIMIT = 5;
private const LIMIT = 5;
private $cachedValues = [];
private $cachedItems = [];
public function run()
{
$values = $this->repeat();
$values[] = 5;
return $values + $this->cachedItems;
}
private function repeat()
{
$items = [];
while ($this->fetch() && $this->fetch() < self::LIMIT) {
$items[] = $this->fetch();
$this->cachedItems[] = $this->fetch();
}
return $items;
}
private function fetch()
{
return mt_rand(1, 15);
}
private function clear()
{
$this->cachedItems = [];
}
}
<?php
final class SomeController
{
- private const MAX_LIMIT = 5;
private const LIMIT = 5;
- private $cachedValues = [];
private $cachedItems = [];
public function run()
{
$values = $this->repeat();
$values[] = 5;
return $values + $this->cachedItems;
}
private function repeat()
{
$items = [];
while ($this->fetch() && $this->fetch() < self::LIMIT) {
$items[] = $this->fetch();
$this->cachedItems[] = $this->fetch();
}
return $items;
}
private function fetch()
{
return mt_rand(1, 15);
}
- private function clear()
- {
- $this->cachedItems = [];
- }
}You're in the Finish!
- How many dead codes did you find?
- Did you just scan right here, because the task was too hard and it didn't make sense to you?
- Would you do it again tomorrow or rather code?
No wonder people don't do code-review (right), there is no time and it's often super boring.
What if there would be a way to automate all that checks above + 10 more with a CI tool. Something that:
- would do code-review for you making your team much smarter forever
- would not require your check on every new piece of code
- you could extend as you like
Have you tried Rector?
Rector doesn't only refactor applications from one framework to another, upgrade your codebase and get you out of legacy. It can be also part of your CI:
composer require rector/rector --dev
vendor/bin/rector process src --set dead-code --dry-runIf Rector detects any dead code, CI will fail. You can, of course, run it without --dry-run after to actually remove the code.
See Dead Code set for more features.