estelsmith / dcp-router Goto Github PK

It would be nice to move to Evenement 2.0 and utilize traits, so that instantiating the router itself is simplified, and not requiring a dependency to be fulfilled.

// Bad.
$emitter = new EventEmitter();
$router = new BaseRouter($emitter);

// Better!!
$router = new BaseRouter();

Currently, BaseRouter has methods for setting and retrieving controller prefixes, even though the methods are only used by MvcRouter.

These methods should be moved into MvcRouter itself, to reduce the footprint of the BaseRouter.

The license should be removed from each code file, and moved into its own LICENSE file, to reduce redundancy across the codebase.

It would be nice if the default controller and default actions were configurable.

class BaseRouter extends MVCRouter
{
    public function __construct()
    {
        parent::__construct();

        $this->setDefaultController('home');
        $this->setDefaultAction('home');
    }
}

$router = new BaseRouter();
$router->dispatch('/'); //Should route to HomeController#homeAction

Currently the README file is horribly out of date, and needs to be updated.

Right now it's fairly difficult to follow the event flow in the codebase. In order to see what events are being called, you need to follow the event chain through the events themselves.

It would be easier to follow events that are being emitted by removing the event-emitting capability of the events themselves, and consolidate into the BaseRouter#dispatch() method.

This will give users a single place to see where things are happening inside the routers.

Currently the CREATE and DISPATCH events contain all the logic required to find and also create/dispatch items. Instead, we should separate the logic of resolving and instantiation into two events. CREATING/DISPATCHING would be for resolving, and CREATE/DISPATCH would be for performing the actual dispatch and/or creation.

This would be useful for when events are overridden to work with DI containers and the like.

The codebase is not compliant with PSR-2 whatsoever. It should be updated to adhere to these standards.

The codebase is lacking any sort of formal tests. Tests should be created to ensure that everything works properly.

There are several docblocks in the project that are out-of-date and missing argument and/or return information.

Methods I've noticed:

• BaseRouter::__construct()
• BaseRouter::convertUrlToArray()
• BaseRouter::dispatch()
• All docblocks in MvcRouter
• RouterInterface::dispatch()

It would be nice to have Travis-CI automatically perform tests against the codebase any time a commit it pushed to the repository.