Small routing system I've written a small routing system in PHP but I'm not sure if it's done the right way and what changes can they be done to the code and improvements.

&lt;?php
namespace Twelve\Router;

    use Twelve\Utils\Utils;
    class Router
    {
        public
            $routeMaps = [],
            $controller;
        private $_route = [];

        public function add($name, $pattern, $controller, array $params = [])
        {
            if(!isset($this-&gt;routeMaps[$name]))
                $this-&gt;routeMaps[$name] =
            [
                'pattern' =&gt; $pattern,
                'controller' =&gt; $controller,
                'params' =&gt; $params,
            ];
        }
        public function findMatch($url)
        {
            foreach($this-&gt;routeMaps as $routeMap)
            {
                $this-&gt;regex = $this-&gt;buildRegex($routeMap['pattern'], $routeMap['params']);
                // Let's test the route.
                if(preg_match($this-&gt;regex, $url))
                {
                    return (array) $routeMap['controller'];
                }
            }
        }
        public function buildRegex($uri, array $params)
        {
            // FInd {params} in URI
            if(preg_match_all('/\{(?:[^\}]+)\}/', $uri, $this-&gt;matches, PREG_SET_ORDER))
            {
                foreach($this-&gt;matches as $isMatch)
                {
                    // Swap {param} with a placeholder
                    $this-&gt;uri = str_replace($isMatch, "%s", $uri);
                }
                // Build final Regex
                $this-&gt;finalRegex = '/^' . preg_quote($this-&gt;uri, '/') . '$/';
                $this-&gt;finalRegex = vsprintf($this-&gt;finalRegex, $params);

                return $this-&gt;finalRegex;
            }
        }
        public function dispatch(array $route = [], $url)
        {
            $this-&gt;setController($route['0']);
            $this-&gt;setAction($this-&gt;getController());
            $this-&gt;setParams(explode('/', $url));
            $this-&gt;controller = $this-&gt;getController();
            array_pop($this-&gt;controller);
            $this-&gt;controller = implode('\\', $this-&gt;controller);
            call_user_func_array([new $this-&gt;controller, $this-&gt;action], $this-&gt;params);
        }

        public function setController($controller)
        {
            $this-&gt;controller = explode(':', $controller);
        }

        public function getController()
        {
            return $this-&gt;controller;
        }
        public function setAction($action)
        {
            $this-&gt;action = end($action);
        }
        public function getAction()
        {
            return $this-&gt;action;
        }

        public function setParams($params)
        {
            $this-&gt;params = array_slice($params, 4);
        }

        public function getParams()
        {
            return $this-&gt;params;
        }
        public function run($uri, array $route = null)
        {
            $route = $this-&gt;findMatch($uri);
            Utils::var_dump($route);
            $this-&gt;dispatch($route, $uri);
        }
    }
Here are my observations

NOTE I did all this via stream-of-conscientiousness and I haven't tested any of these changes (or your original code) so take these as guidance and not exact changes :)

router.php

&lt;?php
class Router
{
    // TODO You need to use `private` a lot more for encapsulation
    private $routeMaps = [];

    // TODO This function needs to document the expected format of inputs
    // TODO is initial controller format supposed to be 'full:class:path:method' ?
    public function add($name, $pattern, $controller, array $params = [])
    {
        // TODO What if it is already set?
        if(!isset($this-&gt;routeMaps[$name]))
        {
            $this-&gt;routeMaps[$name] =
            [
            'pattern'    =&gt; $pattern,
            'controller' =&gt; $controller,
            'params'     =&gt; $params,
            // Build the regex now so we ensure we only build it once
            'regex'      =&gt; $this-&gt;buildRegex($pattern, $params),
            ];
        }
        else
        {
            throw new Exception("Route '{$name}' already defined");
        }
    }

    // TODO Document function

# why the '$route = null'?  It is not used
#?? public function run($uri, array $route = null)
    public function run($uri)
    {
        $routeMap = $this-&gt;findMatch($uri);
#       Utils::var_dump($routeMap);
        // TODO check to see if we actually found a match
        if (null !== $routeMap)
        {
            // TODO we should probably parse the parms before calling dispatch
            $this-&gt;dispatch($routeMap, $uri);
        }
        // TODO probably want to notify user somehow
        else
        {
            ; // ??
        }
    }

    // TODO Why are you calling this parm $uri instead of $pattern?
    private function buildRegex($uri, array $params)
    {
        // FInd {params} in URI
        if(preg_match_all('/\{(?:[^\}]+)\}/', $uri, $matches, PREG_SET_ORDER))
        {
            foreach($matches as $isMatch)
            {
                // Swap {param} with a placeholder
# matching $uri and setting $this-&gt;uri means only the last match is saved
# Is that what you want?  If so, document it
#??             $this-&gt;uri = str_replace($isMatch, "%s", $uri);
                $uri = str_replace($isMatch, "%s", $uri);
            }
            // Build final Regex
            $finalRegex = '/^' . preg_quote($this-&gt;uri, '/') . '$/';
            $finalRegex = vsprintf($finalRegex, $params);

            return $finalRegex;
        }
        // TODO Need to throw error if uri doesn't match
        else
        {
            throw new Exception("Invalid uri: '{$uri}'");
        }

    }

    // TODO Why does (did) this return an array of one element, containing just the
    //      controller?
    // TODO Shouldn't this return the routemap instead of just the controller?
    private function findMatch($url)
    {
        foreach($this-&gt;routeMaps as $routeMap)
        {
            // Let's test the route.
            if(preg_match($routeMap['regex'], $url))
            {
                return $routeMap;
            }
        }

        return null; // TODO We should return something if no match
    }

    // TODO This should take the already parsed $parms    
    private function dispatch($routeMap, $url)
    {
        $parts = explode(':', $routeMap['controller']);
        $action = array_pop($parts);
        $clazz = implode('\\', $parts);
        $params = $this-&gt;parseParams($url);
        call_user_func_array([new $clazz, $action], $params);
    }

    // TODO This needs to be based on the pattern in the rout map
    // TODO Since it appears that the 'params' in the route map have to
    //      match in order for the url to match, we should propably use
    //      the length of those in our parsing here.
    // TODO Perhaps we could even use the regex to tell these values at
    //      match time?
    private function parseParams($url)
    {
        return  array_slice(explode('/', $url), 4);
    }

}