Request handler based solely on reflections ver. 2

Question

This question is connected to the previous one: Request handler based solely on reflections

I have edited the code, taking tim's answer in regard, editing the Kernel class cohesion.

<?php

namespace Qobox;

class Kernel
{
    private $request;
    private $module;

    /**
     * Kernel constructor.
     */
    public function __construct()
    {
        $this->request = $this->getRequest();
        $requestedModuleName = $this->getModule();
        $moduleInstance = $this->getModuleInstance($requestedModuleName);
        $this->setModuleProperties($moduleInstance);
        $moduleMethod = $this->getModuleMethod();
        $argumentArray = $this->setMethodArguments($moduleMethod);
        $moduleMethod->invokeArgs($moduleInstance, $argumentArray);
    }

    /**
     * Grab the Fully Qualified Module Name, including namespace
     *
     * @return string $FullyQualifiedModuleName
     */
    private function getModule()
    {
        $moduleList = require_once "Config/Modules.php";
        /*$moduleList = array(
            "Main" => array(),
            "Nord" => array("Main"),
        );*/
        $namespaceArray = $this->getModuleNamespace($moduleList, array("\\Output"));
        return implode("\\", $namespaceArray);
    }

    /**
     * Get the requested module instance
     *
     * @param $requestedModuleName
     * @return object
     */
    private function getModuleInstance($requestedModuleName)
    {
        try {
            $this->module = new \ReflectionClass($requestedModuleName);
        } catch (\ReflectionException $e) {
            print_r($e);
            exit;
        }

        if ($this->module->isInstantiable() === TRUE) {
            return $this->module->newInstance();
        } else
        {
            // todo: proper user-friendly error handler
            echo 'Module is not instantiable';
            exit;
        }
    }

    /**
     * Get the requested method of the requested module
     *
     * @return mixed
     */
    private function getModuleMethod()
    {
        $moduleMethods = $this->module->getMethods(\ReflectionMethod::IS_PUBLIC);

        foreach ($moduleMethods as $methodKey => $method) {
            if ($method->name == "__construct") {
                unset($moduleMethods[$methodKey]);
            }
        }
        $moduleMethods = array_values($moduleMethods);

        if (!empty($this->request) && $this->module->hasMethod(array_keys($this->request)[0])) {
            // the method exists, load it
            $moduleMethod = $this->module->getMethod(array_shift($this->request));
        } else {
            $moduleMethod = $moduleMethods[0];
        }

        return $moduleMethod;
    }

    /**
     * Combines the full qualified namespace for requested module
     * Recursive function
     *
     * @param array $moduleList
     * @param array $namespaceArray
     * @return array
     */
    private function getModuleNamespace(Array $moduleList, Array $namespaceArray = array())
    {   //print_r(is_array($moduleList[array_keys($moduleList)[0]]));exit;
        if (!empty($this->request) && (array_key_exists(array_keys($this->request)[0], $moduleList) || in_array(array_keys($this->request)[0], $moduleList))) {
            $loader = array_shift($this->request);
        } else if (is_array($moduleList[array_keys($moduleList)[0]])) {
            $loader = array_keys($moduleList)[0];
        } else {
            $loader = array_values($moduleList)[0];
        }

        $namespaceArray[] = $loader;

        if (array_key_exists($loader, $moduleList) && is_array($moduleList[$loader]) && !empty($moduleList[$loader])) {
            return $this->getModuleNamespace($moduleList[$loader], $namespaceArray);
        } else {
            return $namespaceArray;
        }
    }

    /**
     * Get the module properties' instances
     * Recursive function
     *
     * @param array $propertyValue
     * @param String $moduleName
     * @return \stdClass
     */
    private function getModulePropertyInstance(Array $propertyValue, String $moduleName)
    {
        $propertyClass = new \stdClass();
        foreach ($propertyValue as $moduleCaller => $moduleSubname) {
            if (is_array($moduleSubname) === TRUE) {
                $propertyClass->$moduleCaller = $this->getModulePropertyInstance($moduleSubname, $moduleName . "\\" . $moduleCaller);
            } else {
                $propertyClass->$moduleSubname = $this->getSubmodule($moduleName . "\\" . $moduleSubname);
            }
        }

        return $propertyClass;
    }

    /**
     * Get the parsed _$REQUEST array
     *
     * @return array
     */
    private function getRequest()
    {
        $request = array();
        foreach ($_REQUEST as $key => $value) {
            $key = htmlspecialchars($key, ENT_QUOTES | ENT_DISALLOWED | ENT_HTML5, "UTF-8");
            $value = htmlspecialchars($value, ENT_QUOTES | ENT_DISALLOWED | ENT_HTML5, "UTF-8");

            if (empty($value)) {
                $request[$key] = $key;
            } else {
                $request[$key] = $value;
            }
        }

        return $request;
    }

    /**
     * Get submodule instance
     *
     * @param String $submoduleName
     * @return object
     */
    private function getSubmodule(String $submoduleName)
    {
        $namespacePrefixList = array("\\Qobox\\", "\\Modules\\", "\\Output\\");
        foreach ($namespacePrefixList as $namespacePrefix) {
            $submoduleNamespace = $namespacePrefix . $submoduleName;

            try {
                $submodule = new \ReflectionClass($submoduleNamespace);
                break;
            } catch (\ReflectionException $e) {
            }
        }

        $submoduleInstance = $submodule->newInstance();
        $submoduleProperties = $submodule->getProperties(\ReflectionProperty::IS_PROTECTED);

        foreach ($submoduleProperties as $submoduleProperty) {
            $submoduleProperty->setAccessible(true);
            if (empty($submoduleProperty->getValue($submoduleInstance))) {
                $submoduleName = $submoduleProperty->name;
                $submoduleProperty->setValue($submoduleInstance, $this->getSubmodule($submoduleName));
            }
            $submoduleProperty->setAccessible(false);
        }
        return $submoduleInstance;
    }

    /**
     * Set requested module arguments
     *
     * @param $moduleMethod
     * @return array
     */
    private function setMethodArguments($moduleMethod)
    {
        $argumentArray = array();
        $moduleMethodArguments = $moduleMethod->getParameters();
        foreach ($moduleMethodArguments as $Parameter) {
            if (isset($request[$Parameter->name]) && !empty($request[$Parameter->name]) && $request[$Parameter->name] != $Parameter->name) {
                $argumentArray[$Parameter->name] = $request[$Parameter->name];
            } else {
                if ($Parameter->isDefaultValueAvailable() === TRUE) {
                    $argumentArray[$Parameter->name] = $Parameter->getDefaultValue();
                } else {
                    $argumentArray[$Parameter->name] = "";
                }
            }
        }

        return $argumentArray;
    }

    /**
     * Set requested module properties
     *
     * @param $moduleInstance
     */
    private function setModuleProperties($moduleInstance)
    {
        $moduleProperties = $this->module->getProperties();
        foreach ($moduleProperties as $property)
        {
            $moduleName = $property->name;
            $property->setAccessible(true);
            $propertyValue = $property->getValue($moduleInstance);

            if (is_array($propertyValue) === TRUE)
            {
                $propertyClass = $this->getModulePropertyInstance($propertyValue, $moduleName);
                $property->setValue($moduleInstance, $propertyClass);
            } else
            {
                $property->setValue($moduleInstance, $this->getSubmodule($moduleName . "\\" . $moduleName));
            }
        }
    }
}

I intend to leave input sanitation as it is, putting GET and POST variables indistinguishably in one variable, only sanitizing each with htmlspecialchars($key, ENT_QUOTES | ENT_DISALLOWED | ENT_HTML5, "UTF-8"). FILES and SERVER variables are left as it is and are dealt with in another (kernel module) class.

The routing whitelist is put as mock data in the comment in the code. So,

• Quering for index.php?Main will load class /Output/Main/Main.php
• Quering for index.php?Bananaz will load class /Output/Main/Main.php (because Main is loaded by default)

Sure, you could just say that all public methods are, well, public.

Well, yes, only public functions are available, the required logic is in private and protected functions. I don't think users might mess something here up.

An extended example

Output/Example/Example.php

<?php

namespace Output\Main;

class Main
{
    private $Test;
    private $Database;

    public function runExample($hello)
    {
        echo $hello. ', ';
        echo $this->Test->printWorld();
        echo '<br><pre>';
        print_r($this);
    }
}

Modules/Test/Test.php

<?php

namespace Modules\Test;

class Test
{
    public function printWorld()
    {
        return 'World!';
    }
}

Qobox/Database/Database.php

<?php

namespace Qobox\Database;

class Database {
    
    private $Connection;
    private $Statement;
    
    public function __construct() {
        include "Config/Database.php";
        
        try {
            $this->Connection = new \PDO($DatabaseConnect["DBEngine"].":host=".$DatabaseConnect["DBHost"].";dbname=".$DatabaseConnect["DBName"].";charset=utf8", $DatabaseConnect["DBUser"], $DatabaseConnect["DBPass"]);
            $this->Connection->setAttribute(\PDO::ATTR_EMULATE_PREPARES, false); 
            $this->Connection->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
        } catch(\PDOException $e) {
            echo "Connection to the database failed: ".$e->getMessage();
        }
    }
...
}

The output of calling index.php?Example&hello=Hello would be similar to this:

Hello, World
Output\Main Object
(
    [Database:Output\Main:private] => Qobox\Database\Database Object
        (
            [Connection:Qobox\Database\Database:private] => PDO Object
                (
                )

            [Statement:Qobox\Database\Database:private] => 
        )

    [Test:Output\Main:private] => Modules\Test\Test Object
        (
        )
)

Answer 1

Feedback utilized somewhat

It is great that some of the suggestions in the answer by Tim were adopted - e.g. separating the code out into smaller methods, adding PHPDoc comments, etc. The code appears to mostly be in-line with the PSR-12 Standards Recommendations.

The answer by Tim mentioned this about Error handling:

Neither printing exceptions nor ignoring them is a good approach, especially for a low-level class like yours, because you don't allow the calling code to handle the exception like it wants. For example, users will probably want to create a custom error page.

In the original code I see one catch block with a print statement.

   try {
       $moduleReflection = new \ReflectionClass($module);
   } catch (\ReflectionException $e) {
       print_r($e);
   }

In the updated code there are two places where ReflectionExceptions are caught- one in getModuleInstance(), which prints a message, and one in getSubmodule() - the latter being inside of a loop and appears to ignore the exception so the loop can continue until it finds the reflection it needs.

As a developer that would use this framework with developer or output modules it would seem natural that the framework should not catch the exceptions

Eliminating else keyword

There are a few places where the else keyword can be eliminated - typically following a return statement in a conditional block. For example - in getModuleInstance() the last blocks are as follows:

    if ($this->module->isInstantiable() === TRUE) {
        return $this->module->newInstance();
    } else
    {
        // todo: proper user-friendly error handler
        echo 'Module is not instantiable';
        exit;
    }

This could be simplified like so:

    if ($this->module->isInstantiable() === TRUE) {
        return $this->module->newInstance();
    }
    // todo: proper user-friendly error handler
    echo 'Module is not instantiable';
    exit;

Also in getModuleMethod() the final lines are:

    if (!empty($this->request) && $this->module->hasMethod(array_keys($this->request)[0])) {
        // the method exists, load it
        $moduleMethod = $this->module->getMethod(array_shift($this->request));
    } else {
        $moduleMethod = $moduleMethods[0];
    }

    return $moduleMethod;

There is little point in assigning moduleMethod (unless it was useful for debugging):

    if (!empty($this->request) && $this->module->hasMethod(array_keys($this->request)[0])) {
        // the method exists, load it
        return $this->module->getMethod(array_shift($this->request));
    } 
    return $moduleMethods[0];

Also at the end of getModuleNamespace():

if (array_key_exists($loader, $moduleList) && is_array($moduleList[$loader]) && !empty($moduleList[$loader])) {
    return $this->getModuleNamespace($moduleList[$loader], $namespaceArray);
} else {
    return $namespaceArray;
}

Again the else can be eliminated:

if (array_key_exists($loader, $moduleList) && is_array($moduleList[$loader]) && !empty($moduleList[$loader])) {
    return $this->getModuleNamespace($moduleList[$loader], $namespaceArray);
}
return $namespaceArray;

Identical Comparisons with TRUE

In the method getModulePropertyInstance() is this conditional:

        if (is_array($moduleSubname) === TRUE) {

The function is_array() returns a bool¹ and thus that conditional can be simplified to:

        if (is_array($moduleSubname)) {

Simplifying request array assignment

The block within the foreach in getRequest() is currently:

        if (empty($value)) {
            $request[$key] = $key;
        } else {
            $request[$key] = $value;
        }

That could be simplified using a ternary operator:

        $request[$key] = $value ? $value : $key;

As I mentioned in this answer the ternary short cut ?: , A.K.A. "the Elvis operator" has been available since PHP 5.3 - it could simplify that line even more:

        $request[$key] = $value ?: $key;