I'm working with an open source project and thought it would be a good idea to implement automated code revisions with phpmd.
It showed me many coding mistakes that im fixing already. But one of them made me curious.
Consider the following method:
/**
*
* @param string $pluginName
*/
public static function loadPlugin($pluginName){
$path = self::getPath()."plugins/$pluginName/";
$bootPath = $path.'boot.php';
if(\is_dir($path)){
//Autoload classes
self::$classloader->add("", $path);
//If theres a "boot.php", run it
if(is_file($bootPath)){
require $bootPath;
}
}else{
throw new \Exception("Plugin not found: $pluginName");
}
}
Here, phpmd says that Else is never necessary
...An if expression with an else branch is never necessary. You can rewrite the conditions in a way that the else is not necessary and the code becomes simpler to read. ...
is_dir
will return false whenever the given path is a file or simply does not exist, so, in my opinion, this test is not valid at all.
Is there a way to fix it or maybe simply ignore cases like this one?
An alternative to the structure is something like this:
public static function loadPlugin( $pluginName ) {
$path = self::getPath() . "plugins/$pluginName/";
$bootPath = $path . 'boot.php';
if( \is_dir( $path ) ) {
// Autoload classes
self::$classloader->add( "", $path );
// If theres a "boot.php", run it
if ( is_file( $bootPath ) ) {
require $bootPath;
}
// A return here gets us out of the function, removing the need for an "else" statement
return;
}
throw new \Exception( "Plugin not found: $pluginName" );
}
While I'm not sure it's the solution, it is a technique to avoid the else
condition. Else conditions can add complexity when trying to read the code, and allowing the function to "flow" without else conditions can make them more readable.