phpcoding-styleconventionsphpmd

PHP Mess Detector giving false positives


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?


Solution

  • 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.