Дали мне тестовое задание, я его сделал за 8 часов. Помогите найти косяки, не считая токенов. CRUD для управления пользователями. У каждого пользователя должны быть следующие поля: логин, пароль, ФИО, E-Mail, роль В поле E-Mail необходимо разрешить вводить только валидный E-Mail адрес Администратор должен иметь возможность просматривать список пользователей, добавлять новых, редактировать и удалять существующих Пользователь должен иметь возможность просматривать список пользователей, просматривать профиль выбранного пользователя и редактировать свой профиль. На странице со списком пользователей должна присутствовать возможность сортировки и фильтрации по логину, ФИО и роли Залил на гитхаб, там есть и sql файл https://github.com/Div-Man/grud
https://github.com/Div-Man/grud/blob/master/index.php error_reporting(E_ALL & ~E_NOTICE & ~E_DEPRECATED & ~E_STRICT); ini_set('display_errors', 1); достаточно error_reporting(E_ALL); в пхп 7 E_STRICT включен в E_ALL PHP: $db = DataBase::connect( ...$config['mysql'] ); --- Добавлено --- https://github.com/Div-Man/grud/blob/master/lib/database/DataBase.php http://phpfaq.ru/pdo/pdo_wrapper --- Добавлено --- https://github.com/Div-Man/grud/blob/master/lib/router/router.php 3 строка ошибка. Или меня уже глючит и инклуд на столько умный что ищет папку контроллера по всему сайту, после нахождения идет дальше по пути. --- Добавлено --- https://github.com/Div-Man/grud/blob/master/model/users.php $query->setFetchMode(PDO::FETCH_ASSOC); что там забыла ? у тебя по дефолту объявлено уже как ассоциативный массив, + PDO::FETCH_ASSOC суется в fetch и fetchAll, setFetchMode юзается по другой надобности --- Добавлено --- https://github.com/Div-Man/grud/blob/master/lib/router/router.php с 25 строки нотисы могут быть --- Добавлено --- вобщем забью. --- Добавлено --- error_reporting(E_ALL & ~E_NOTICE & ~E_DEPRECATED & ~E_STRICT); включить все ошибки кроме: нотисов депрекаты типы все трое скрыты. Молодец
https://github.com/Div-Man/grud/blob/master/lib/router/router.php - это жуть, а не роутер. У роутера должна быть задача - найти нужный контроллер и передать ему управление, всё. А у тебя он какие-то кнопки проверяет, в $_POST лезет, и т.п. сделан не универсально. Надо такие общие вещи писать так, чтоб можно было выдернуть в другой проект. А что, фреймворки вам сразу запретили использовать?
@romach, хорошая шутка. --- Добавлено --- @Dimon2x, а почему методы в модели называются showTable, showUser? Они же ничего не показывают, они читают записи из базы. Так и обзови: getUsers, getUser, или readUser, а то вводишь читателя в заблуждение
@Dimon2x не парься и заюзай slim framework --- Добавлено --- PHP: private function render($template = null, $params = null) { $fileTemplate = 'template/'.$template; if (is_file($fileTemplate)) { ob_start(); if (count($params) > 0) { extract($params); } include $fileTemplate; return ob_get_clean(); } } Лучше используй file_exists для проверки файла --- Добавлено --- но роутер конечно у тебя забавный. --- Добавлено --- Он не просто не универсален. Это роутером назвать то сложно. Это скорее некий контроллер, который почему то в больном уме разраба назвался роутером.
Чтобы твой самописный фреймворк был так же надёжен, как Yii, к примеру, твоя квалификация должна быть сравнима с квалификацией его автора Александра Макарова. Это явно не случай ТС (без обид). И от горячего пропагандиста Laravel @romach (у меня, по крайней мере, такое пока впечатление создалось) это звучало шуткой. --- Добавлено --- Laravel, конечно, для текущей задачи много, но какой-нибудь Slim или F3 я бы взял.
https://github.com/Div-Man/grud/blob/master/controller/UsersController.php Разный стиль наименования, где-то с большой буквы, где-то с малой. Не надо так. https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L3 https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L7 Не надо тянуть за собой зависимости, надо изобрести DI и IoC https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L8 Почему предпоследний аргумент имеет дефолтное значение, а последний нет? https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L36 А если в форму регистрации добавится ещё несколько полей? Монитор не резиновый. https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L37 Почему у модели show, а контроллера get? Первая ведь получает, а второй - показывает. В контроллерах бывает используют префикс методов get/post для наглядности, да, но это другое. https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L54 А почему часть валидации в контроллере, а часть унесена в модель? https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L83 Воу, откуда в модели html? Модель про данные, не про валидацию, плевание хтмлками и даже не про БД, если уж на то пошло )) https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L83 Почему где-то мы параметры биндим, а где-то так вставляем? Как потом различать, что вот тут можно, а тут вася-программист обленился и так сойдет? ) https://github.com/Div-Man/grud/blob/master/controller/UsersController.php#L83 Так. Делать. Нельзя. Модель не должна убивать приложение потому что ей так захотелось - это не её зона ответственности. https://github.com/Div-Man/grud/blob/master/model/users.php#L57 Ну и я не знаю конечно требований работодателя, но PSR-2 / PSR-4 должно быть, а ещё было бы плюсом использование сторонних пакетов там где это ускоряет разработку и не связано с ТЗ: fastRoute, monolog, раз запилил логирование, ну или микрофреймворк типа silex/slim и что там нынче ещё есть. Резюмируя: вообще, список можно продолжать и дальше, но думаю суть вы уловили. Никогда не глушите ошибки, вообще никакие, ознакомьтесь с code style и стандартами, разберитесь с autoload, namespace, научитесь использовать готовые инструменты для типовых задач, покурите микрофреймворки или просто документацию к популярным фреймам, попробовав понять почему оно было реализовано именно так и всё будет нормально )