1
\$\begingroup\$

I've created a library, TomHart/laravel-route-from-model, and looking to get a review on it. As well as the usual code review, I'm also looking for feedback from a users point of view, if you were to use the library, is there anything extra you wish it did, anything different etc.

The key class is:

<?php
namespace TomHart\Routing;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Routing\Router;
use Illuminate\Routing\UrlGenerator;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
class RouteBuilder
{
    /**
     * Get the router instance.
     *
     * @return Router
     */
    private function getRouter(): Router
    {
        return app('router');
    }
    /**
     * Get the UrlGenerator.
     *
     * @return UrlGenerator
     */
    private function getUrlGenerator(): UrlGenerator
    {
        return app('url');
    }
    /**
     * This allows a route to be dynamically built just from a Model instance.
     * Imagine a route called "test":
     *      '/test/{name}/{id}'
     * Calling:
     *      routeFromModel('test', Site::find(8));
     * will successfully build the route, as "name" and "id" are both attributes on the Site model.
     *
     * Further more, once using routeFromModel, the route can be changed. Without changing the call:
     *      routeFromModel('test', Site::find(8));
     * You can change the route to be:
     *      '/test/{name}/{id}/{parent->relationship->value}/{slug}/{otherParent->value}'
     * And the route will successfully change, as all the extra parts can be extracted from the Model.
     * Relationships can be called and/or chained with "->" (Imagine Model is a Order):
     *      {customer->address->postcode}
     * Would get the postcode of the customer who owns the order.
     *
     * @param string  $routeName The route you want to build
     * @param Model   $model     The model to pull the data from
     * @param mixed[] $data      Data to build into the route when it doesn't exist on the model
     *
     * @return string The built URL.
     */
    public function routeFromModel(string $routeName, Model $model, array $data = [])
    {
        $router = $this->getRouter();
        $urlGen = $this->getUrlGenerator();
        $route = $router->getRoutes()->getByName($routeName);
        if (!$route) {
            throw new RouteNotFoundException("Route $routeName not found");
        }
        $params = $route->parameterNames();
        foreach ($params as $name) {
            if (isset($data[$name])) {
                continue;
            }
            $root = $model;
            // Split the name on -> so we can set URL parts from relationships.
            $exploded = collect(explode('->', $name));
            // Remove the last one, this is the attribute we actually want to get.
            $last = $exploded->pop();
            // Change the $root to be whatever relationship in necessary.
            foreach ($exploded as $part) {
                $root = $root->$part;
            }
            // Get the value.
            $data[$name] = $root->$last;
        }
        return rtrim($urlGen->route($routeName, $data), '?');
    }
}
\$\endgroup\$
1
\$\begingroup\$

Laravel coupling

What a shame that you decided to tie this to laravel. You can decouple the entire library from laravel framework and only provide a bundle for laravel.

routeFromModel accepts Model, but it can actually work for any object.

$route = $router->getRoutes()->getByName($routeName);
if (!$route) {
  throw new RouteNotFoundException("Route $routeName not found");
}
$params = $route->parameterNames();

This means you really don't need the router, you just need something that gives you an "array of prameter names" based on a "name".

return rtrim($urlGen->route($routeName, $data), '?');

Returning just the data here would make it more flexible.

IoC

You are pulling the RouteBuilder from DI container, why not have the container inject those deps. The way it is now, it could just be a static class with only static methods...

\$\endgroup\$
2
  • \$\begingroup\$ Ahh interesting! I never thought of it as anything but Laravel, but yeah that makes sense. I'll try and decouple it as an exercise, see what can be done, thanks :)! \$\endgroup\$ – TMH Dec 10 '19 at 11:44
  • \$\begingroup\$ I did as you recommended, pulled the core logic into it's own repo, and made use of that in the laravel specific library. \$\endgroup\$ – TMH Dec 10 '19 at 18:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.