MVC маршрутизатор на PHP — есть ли изъяны?

Question

[onogur]

Написал MVC маршрутизатор на PHP. Хотелось бы вынести на суд общественности, есть какие-либо изъяны?

На данный момент он работает отлично.
https://github.com/vaaslee/php

Answer 1

[Артур]

и судя по коду Вы его оторвали от самописного движка/фреймворка, в котором Вы отказались следовать каким-либо PSR и прочим стандартам, не говоря уже о каше в коде :)

Comments

[onogur]

Писал все сам,PSR активно изучаю)Просто опыта не много.Хотелось бы знать про дырки в безопасности

[Артур]

Посмотрите, например, на Klein. https://github.com/chriso/klein.php

Answer 2

[Alex Babak]

На данный момент он работает отлично -- всё отлично :) нафиг тогда вам суд общественности?

Answer 3

[Назар Мокринский]

Не понятно, что за переменная $URL
И что за элементы в массиве. Можно догадаться, что это каким-то образом распарсенные части адреса, но точно не понятно. Допишите как минимум PhpDoc секции.

Comments

[onogur]

Из FrontController в Router передается разбитый по разделителям URL.

[Назар Мокринский]

@onogur Вот добавьте строчку PhpDoc:
@param string[] $URL Url splitted by /
И будет всё понятно, и IDE документацию подхватит где нужно. А вообще - почему не разбивать строку здесь? Вроде логичнее - на входе привычный URL, по коду будет понятно, что строка разбивается и создается массив.

[Константин Андреевич]

В роутер ничего не надо передавать, он сам способен получить всё из адресной строки и разобрать.

[Назар Мокринский]

@reffy Не факт. Есть вероятность, что нужно будет распарсить строку, которая пришла не в адресе. + для тестов удобнее не подменять системные переменные, а передавать их. Но это сильно зависит от того, что за система предполагается в результате. А этого из одного класса автора узнать невозможно.

Answer 4

[Константин Андреевич]

Такой маршрутизатор писали наверное все :) Вот например на хабре статья была хорошая — habrahabr.ru/post/150267

А по делу мне стало интересно пару моментов:
— зачем нужно определять $viewFile?
— у вас используется именование классов controllerName? А почему не CamelCase?
— зачем всегда делать исключение и выдавать ошибку? Почему не вызвать 404?
— роутер разбирает только 3 уровня вложенности? /url1/url2/param. А если уровней больше? Например /admin/blog/edit/2.
— у вас обязательно использование модели. А модели может и не быть. Например SiteController отображает статические страницы и не имеет модели.

Comments

[onogur]

-$viewFIle мое упущение)Он не нужен.
-Возьму на заметку
-Описать класс с ошибкой 404?
-Роутер писался под нынешний проект,поэтому так, но я думаю,можно добавить еще один параметр.
-А что если подключение модели воткнуть в file_exists?

[Назар Мокринский]

@reffy Camel - переводится как верблюд. То есть camelCase, а то что вы написали - это PascalCase.

[Константин Андреевич]

@nazarpc вы не знаете что такое Google? ru.wikipedia.org/wiki/CamelCase

[Назар Мокринский]

@reffy По ссылке что сами прислали упоминаются UpperCamelCase и lowerCamelCase.
Но если почитать en.wikipedia.org/wiki/CamelCase то можно обнаружить:
> Although the first letter of a camel case compound word may or may not be capitalized, the term camel case generally implies lowercase first letter. For clarity, this article calls the two alternatives upper camel case and lower camel case. Some people and organizations use the term camel case only for lower camel case. Other synonyms include:...

То есть обычно принято использовать первую букву маленькой, а для первой большой используется термин PascalCase, чтобы не путать и объясняться однозначно.

[Назар Мокринский]

@reffy И да, в PHP принято в большинстве для методов использовать camelCase, только не такой, как вы написали.

Answer 5

[onogur]

Мой код - быдлокод?

Comments

[Константин Андреевич]

Я бы сказал у вас код новичка. Я помню точно так же начинал писать роутер. Затем, он стал обрастать всякими фишками, типа ошибки 404, передача неограниченного числа параметров контроллеру, определение поддоменов(у вас сейчас можно обратиться к любому поддомену и это будет аналогом site.ru), и т.д.

Ставьте себе задачу, реализуйте, изучайте другие решения.