Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does clean code forbid else expression

Tags:

php

phpmd

I have this code in a function:

if ($route !== null) { // a route was found
    $route->dispatch();
} else {
    // show 404 page
    $this->showErrorPage(404);
}

Now PHPmd gives an error:

The method run uses an else expression. Else is never necessary and you can simplify the code to work without else.

Now I'm wondering if it really would be better code to avoid the else and just add a return statement to the if part?

like image 454
Pascal Avatar asked Sep 20 '15 07:09

Pascal


3 Answers

PHPMD is expecting you to use an early return statement to avoid the else block. Something like the following.

function foo($access) 
{
    if ($access) {
        return true;
    }

    return false;
}

You can suppress this warning by adding the following to your class doc block.

/**
 * @SuppressWarnings(PHPMD.ElseExpression)
 */
like image 173
Abin Avatar answered Nov 08 '22 02:11

Abin


You usually can rewrite the expression to use just an if and it does subjectively make the code more readable.

For example, this code will behave in the same way if showErrorPage breaks the execution of the code.

if ($route == null) { 

   $this->showErrorPage(404);
} 
$route->dispatch();

If the content of your if statement does not break the execution, you could add a return

if ($route == null) { 

   $this->showErrorPage(404);
   return;
} 
$route->dispatch();

If you where inside a loop, you could skip that iteration using continue

    foreach ($things as $thing ) {
        if ($thing == null) {
            //do stuff and skip loop iteration
            continue;
        }     

        //Things written from this point on act as "else"

    }
like image 22
Joaquin Brandan Avatar answered Nov 08 '22 02:11

Joaquin Brandan


I wouldn't worry about what PHPmd says , atleast in this case.

They probably meant for you to use the conditional operator because (in their opinion) its 'cleaner'.

$route !== null  ?  $route->dispatch() : $this->showErrorPage(404) ;
like image 2
gyaani_guy Avatar answered Nov 08 '22 03:11

gyaani_guy