Am I implementing $router->respond() properly?

[funkytaco]

Hello,

I am following a no-framework tutorial and am using Klein as the router.

In my bootstrap.php file:

$router = new \Klein\Klein();
$routes = include('Routes.php');
foreach ($routes as $route) {
    $router->respond($route[0], $route[1], $route[2]);
}

In my Routes.php file: return [ ['GET', '/hello-world', function () { echo 'Hello World'; }], ['GET', '/another-route', function () { echo 'This works too'; }], ['GET', '/', ['Main\Controllers\HomeController', 'show']], ];

This works, but it's calling HomeController::show() as a static function. I guess that makes sense, but then how do I use __construct() in my HomeController to setup the class?

[Rican7]

The third parameter in your passing is a callable.

PHP allows for class/object callables to be denoted via an array syntax, as you have in your ['Main\Controllers\HomeController', 'show'] example.

Since you're passing a class-name string as the first part of the array, it will call the method statically. If you'd like to call the the show() method in reference to an object instance, simply change the array callable to an instance and method, like this: [$instance_of_home_controller, 'show'].

[funkytaco]

Wow, you are fast.

I'm used to PHP 5.2, so new to 5.3 way of doing things. This seems to work:

return [
    ['GET', '/hello-world', function () {
        echo 'Hello World';
    }],
    ['GET', '/another-route', function () {
        echo 'This works too';
    }],
    ['GET', '/', [new Main\Controllers\HomeController(new \Nette\Http\Response()), 'show']],

];

Thank you, @Rican7.

[funkytaco]

Even better, I also just noticed in the README.md that klein passes a $request and $response objects to the 'show' method for my HomeController route, so I don't know why I would even need Nette's HTTP response.

[Rican7]

Haha, I'm glad I could help! :)

And yea, I wouldn't use Nette's HTTP response class, as using Klein's allows you to hook more into Klein's routing behavior. Its also usually a better idea to have controllers constructors take in any domain-specific services or repositories in the constructor and have the controller's route matched methods take in the HTTP request/response. Does that make sense?

Something like this, perhaps:

class UserController
{
    private $user_repository;
    private $registration_service;
    private $model_serializer;

    public function __construct(UserRepository $user_repository, RegistrationService $registration_service, ModelSerializer $model_serializer)
    {
        $this->user_repository = $user_repository;
        $this->registration_service = $registration_service;
        $this->model_serializer = $model_serializer;
    }

    public function get(Request $request, Response $response)
    {
        $user_model = $this->user_repository->findById(
            $request->param('id')
        );

        if (null === $user_model) {
            $response->code(404);
        } else {
            $response->body(
                $this->model_serializer->serialize($user_model)
            );
        }

        return $response;
    }

    public function register(Request $request, Response $response)
    {
        try {
            $user_model = $this->registration_service->registerByParamArray(
                $request->params
            );
        } catch (RegistrationException $e) {
            // Handle error
            $response->code(400);
            $response->body($e->getErrorParams());
        }

        if (null !== $user_model) {
            $response->body(
                $this->model_serializer->serialize($user_model)
            );
        }

        return $response;
    }
}

// In routes file....

$router = new Klein\Klein();
$app = new My\Awesome\App();
$controller_factory = new ControllerFactory($app);

// Group some calls under the `/users` route namespace
$router->with('/users', function () use ($router, $controller_factory) {
    $controller = null;

    // Catch-all for this group to build the controller.
    // Make sure to pass in a REFERENCE to the controller variable!
    $router->respond(function () use (&$controller, $controller_factory) {
        $controller = $controller_factory->buildUserController();
    });

    // GET /users/?
    $router->respond('GET', '/?', [$controller, 'get']);

    // POST /users/?
    $router->respond('POST', '/?', [$controller, 'register']);
});

.... Oh god, sorry for that long response, haha. I kinda got into it... Anyway, I hope that helps (I'm really hoping I didn't just confuse you more...).

PS: I haven't tested or ran any of that code.... so it might not even work. :P

[funkytaco]

I've been away from PHP a bit (Code Igniter was the big guy back then for anything close to this), as we've started moving towards Javascript and Java on the backend, and I see some similarities here on the code separation mentality. I don't remember the community caring about that before. It's as if PHP is finally becoming more nimbly used by the community instead of being used as a blunt object. I like it.

I'm going to go through this and take some time to digest it. Thanks for the info.

[Rican7]

Haha, yea, PHP's come along way. Some outside of the community still see it as the bruised old haphazard language, haha, but there's been a huge maturing of the community. Its strong object-oriented model introduced in PHP 5+ helped a lot with that. :smile:

(In fact, this Klein library itself could use some re-factoring for better separation of concerns, but that's in the works. :wink:)

Sorry if my response was a bit overdone or complex, but I saw that you were attempting to attack a controller-separation strategy and I figured I'd help with fleshing out a potential evolution of that strategy.

Let me know if you need any more help. )

[funkytaco]

I can't figure out what $controller_factory->buildUserController() is supposed to do. I tried to get it to return a new controller, but I get Expected a callable. Got an uncallable array klein/klein/src/Klein/Route.php line 124.

If I just hardcode the $controller variable without using the factory, the routes respond as expected.

[Rican7]

Ahhhh, I see what the problem is. See, I told you I didn't run the code before posting it. :P

But yea, the problem is that the respond() method expects a callable, but since the controller is lazily instantiated the $controller variable is still null by the time its passed into the other respond() methods. Hmm....

You could technically build the controller in the with() block without a catch-all, but that means that you're controller building process will happen on each request, regardless of whether the with group matches (the with() blocks respond methods are called immediately during, they aren't called upon matching... yea, I know, not ideal).

So, hmm, I guess you could technically create some sort of ControllerInvoker class that takes in all of your controller factories and then builds and invokes those methods, lazily.... something like this maybe?:

<?php

/**
 * ControllerFactory
 *
 * Defines an interface for how controller factories should build based on dynamic names
 */
interface ControllerFactory
{
    public function build($name);
}

/**
 * ControllerInvoker
 *
 * Invokes controllers lazily by containing instances and proxying calls
 */
class ControllerInvoker
{

    /**
     * @type string
     */
    const CONTROLLER_METHOD_SEPARATOR = '|';

    /**
     * The factory class used to build
     *
     * @type mixed
     */
    private $controller_factory;

    /**
     * A map of controller names (key) to their corresponding controller instances (values)
     *
     * @type array
     */
    private $controller_map = [];

    public function __construct(ControllerFactory $controller_factory)
    {
        $this->controller_factory = $controller_factory;
    }

    public function getController($name)
    {
        // If it hasn't been built yet
        if (empty($this->controller_map[$name])) {
            // Build and store for the next call
            $this->controller_map[$name] = $this->controller_factory->build($name);
        }

        return $this->controller_map[$name];
    }

    public function setController($name, $controller)
    {
        $this->controller_map[$name] = $controller;
    }

    /**
     * Build our "magic" string to have be called later and proxied to our controllers
     *
     * @param string $controller_name
     * @param string $method_name
     * @return string
     */
    public static function buildCallString($controller_name, $method_name)
    {
        return $controller_name . self::CONTROLLER_METHOD_SEPARATOR . $method_name;
    }

    /**
     * Get a `callable` by processing a call string
     *
     * @param string $call_string
     * @return callable
     */
    private function getCallableForCallString($call_string)
    {
        list($controller_name, $method_name) = explode(CONTROLLER_METHOD_SEPARATOR, $call_string);

        return [$this->getController($controller_name), $method_name];
    }

    /**
     * Process the magic!
     *
     * @param string $call_string
     * @param array $args
     * @return mixed
     */
    public function __call($call_string, $args)
    {
        return call_user_func_array(
            $this->getCallableForCallString($call_string),
            $args
        );
    }
}

// In routes file....

$router = new Klein\Klein();
$app = new My\Awesome\App();
$controller_factory = new ControllerFactory($app);
$invoker = new ControllerInvoker($controller_factory);

// Group some calls under the `/users` route namespace
$router->with('/users', function () use ($router, $invoker) {

    $group_controller_name = 'users';

    // GET /users/?
    $router->respond(
        'GET',
        '/?',
        [$invoker, $invoker->buildCallString($group_controller_name, 'get')]
    );

    // POST /users/?
    $router->respond(
        'POST',
        '/?',
        [$invoker, $invoker->buildCallString($group_controller_name, 'register')]
);
});

So this would essentially give the Route a callable to call later, which the invoker would then build the controller and proxy the call to.

.... Again, none of this is tested (sorry), and its a bit magical... but the more I look at it, the more I like it. :stuck_out_tongue_closed_eyes:

Sorry if I've only further complicated matters even more...

[alexschwarz89]

Hello there… I also wanted to keep my routes in the Controllers and ended up with this solution. I define the routes in an Array of each Controller and the index file should load them. This is actually working, I am just interested what you think about it. Feel free to criticize.

$controllerMap = ['user' => 'UserController'];

foreach($controllerMap as $namespace=>$controllerName) {
    $controllerName = '\\Controller\\' . $controllerName;
    $controller = new $controllerName;

    $klein->with("/$namespace", function () use ($klein, $controller) {
        foreach ($controller->routes as $route) {
            $klein->respond($route[0], $route[1], [$controller, $route[2]]);
        }
    });
}

And in UserController looks like

    public $routes = [
        ['GET', '/edit', 'edit']
    ];

Greetings

[funkytaco]

Hi @alexschwarz89,

I like the idea of putting the routes in the Controller. But, I also like how I have all of my routes in one spot. I'm not sure which is better. I guess having it in the controller makes more sense, because then I can drop in a controller file.

For my setup below, Routes.php just returns an array.

$routes = include(SOURCE_DIR . '/Routes.php');

foreach ($routes as $route) {
        $router->respond($route[0], $route[1], $route[2]);
}

$router->dispatch();

Also, how are you handling file mime types (i.e. .js,.xml,.json) if your routes are in the controllers? I guess you could setup a controller for the mime types.