Какие проблемы могут возникнуть: router?

Question

[Aleksandr]

Доброго времени!

Собственно весь вопрос выше :-)

class Router
{
...
...
...
    private function __clone() {}
    private function __construct() {}
    
    public static function getInstance()
    {
        if (null === self::$instance) {
            self::$instance = new self();
        }
        return self::$instance;
    }

    public function init()
    {
        $this->_getUrl();

        if (empty($this->_url[0])) {
            $this->_loadDefaultController();
            return false;
        }

        $this->_loadExistingController();
        $this->_callControllerMethod();
        return true;
    }
...
...
...

Comments

[Котик Антон]

С чего вообще взяли, что могут возникнуть проблемы? Вопрос конкретно в чём?

[Aleksandr]

в синглтоне

Answer 1

[Виталий Хоменко]

Синглтон там явно лишний. И если роут не определен отображать 404, а не дефолтный контроллер.

Добавлено:

> про дефолтный контроллер чуть подробнее

При запросе "/такой-страницы-у-меня-нет" нужно показать 404. А в вашем же случае все ссылки на несуществующие страницы будут отображать дефолтный контроллер.

> Объясните пожалуйста почему синглтон лишний

Синглтон нужен для получения доступа к инстансу с любого места в приложении. Если бы это был главный контейнер приложения, то да, это имеет смысл. Но для роутера, который выполняется всегда в одном и том же месте в приложении, синглтон не нужен. Вы просто раз вызвали роутер и больше он не нужен: ( new Router() )->init();

Comments

[Aleksandr]

Объясните пожалуйста почему синглтон лишний, и про дефолтный контроллер чуть подробнее

[Aleksandr]

OnYourLips: каие?

[Виталий Хоменко]

Aleksandr
> про дефолтный контроллер чуть подробнее

При запросе "/такой-страницы-у-меня-нет" нужно показать 404. А в вашем же случае все ссылки на несуществующие страницы будут отображать дефолтный контроллер.

> Объясните пожалуйста почему синглтон лишний

Синглтон нужен для получения доступа к инстансу с любого места в приложении. Если бы это был главный контейнер приложения, то да, это имеет смысл. Но для роутера, который выполняется всегда в одном и том же месте в приложении, синглтон не нужен. Вы просто раз вызвали роутер и больше он не нужен: ( new Router() )->init();

[Aleksandr]

> Синглтон нужен для получения доступа к инстансу с любого места в приложении...

Но ведь тогда при каждом $_GET, $Router = new App\Router(); будет создавать новый объект, или я что-то недопонимаю?

[Aleksandr]

Я его уже убрал ;-) Так как он действительно был лишний. Спасибо за направление

Answer 2

[Mikhail Osher]

Зачем писать велосипеды, когда есть замечательный FastRoute?

Comments

[Aleksandr]

даже не знал про такой

Answer 3

[Матвей Мамонов]

А где описание функции _getUrl() ?
Либо я слепой, либо вы его замаскировали под этими троеточиями.
Просто, если у вас url делится по слешам, то я знаю, где может затаиться ошибка (вангую).

Comments

[Aleksandr]

private function _getUrl()
{
$url = isset($_GET['url']) ? $_GET['url'] : null;
$url = rtrim($url, '/');
$url = filter_var($url, FILTER_SANITIZE_URL);
$this->_url = explode('/', $url);
}

[Матвей Мамонов]

Aleksandr:
А почему $_GET['url']?
Почему не $_SERVER['REQUEST_URI']?
Просто дело в том, что если будет введен адрес типа
example.com/param1/param2//param4
то тот, что 3-ий по счету будет равен null.

Если Вы реализуете что-то типа концепции MVC, то это может к неожиданным результатам привести. Ну, если пользователь 2 слеша введет.

[Aleksandr]

Да MVC, $_GET и $_SERVER['REQUEST_URI'] это разве неодно и тоже? Просто опыта нет.

Что касается example.com/param1/param2//param4 то если есть лишний / (или несколько) без разницы в каком мести, то всё работает как и должно, и параметры не теряются. Скорее всего filter_var их сама лишние убирает

[Матвей Мамонов]

Aleksandr: Да, верно, я в свое время это упустил.
Но что ж, сразу вы код не выложили, поэтому я мог делать только догадки. А теперь все стало видно, и вроде бы все работает. Удачи :)