Я специально пишу тут не только ответ, но и вопрос: на случай, если кому-то еще интересно посмотреть код, или дать какой-то дополнительный совет.
----
Вопрос>>956975 |
http://arhivach.org/thread/245785/#956975Я тут уже всплывал со своим говноблогом, пытаюсь вот его переписать согласно mvc.
В общем, я написал такой контроллер, в нем стартует сессия и инстанцируются общие для всех страниц классы.
https://github.com/honeydev/s-blog/blob/master/app/Controllers/MainController.phpЭтот класс наследуют все остальные классы контроллеры. Которые в свою очередь уже запускаются роутером. Все ли я правильно делаю?
ОтветДумаю. что неправильно. Ну например, почему в контроллере идет работа с какими-то сессиями? это задача сервиса, отвечающего за авторизацию. Также, сессии плохо подходят для авторизации так как удаляются при неактивности в течение некоторого времени, около получаса.
Вообще, мне кажется неправильно в конструкторе заполнять какие-то массивы данными. А вдруг ничего выводиться не будет? зачем их было получать? Лучше в функции render() получать и добавлять эти данные.
Вообще, контроллеру в конструктор обычно передают DI контейнер, из которого он и берет экземпляры сервисов. В уроке про DI написано. Так удобнее, и гарантируется что будет испльзоваться один общий экземпляр сервиса.
В общем, лучше просто ограничиться получением нужных сервисов, а потом уже в render либо их передавать либо данные их них передавать во вью.
> Set assoc array with keys title, author, email
Вместо того, чтобы делать массив, удобнее просто передать сервис и пусть вью берет что ему нужно. Ну или передавать это как отдельные элементы. А ты зачем-то все передаешь через массив. Неудобно же, по твоей логике, надо все методы убрать и оставить только один, возвращающий огромный массив со всеми данными сразу?
Лучше просто сделать getTitle(), getAuthor и так далее.
У сервиса нет задачи выдавать данные специально для вью, а ты явно делаешь метод в сервисе, чтобы быстрее передать данные во вью одним массивом.
https://github.com/honeydev/s-blog/blob/master/app/Services/AdminService.phpНе очень понятно назначение класса.
https://github.com/honeydev/s-blog/blob/master/tests/forsanitize.phpЭто тестировать удобнее не наколенными скриптами, а юнит-тестами под phpunit. codeception кстати содержит в себе phpunit.
Все твои сервисы удобно тестировать именно юнит-тестами. Снаружи, через интерфейс, их тестировать будет не удобно.
https://github.com/honeydev/s-blog/blob/master/tests/tests/acceptance/ChangeRegistrationStatusCept.phpТут много копипасты. Если у тебя есть специальный тестовый аккаунт гораздо логичнее в тестовом хелпере сделать один метод для залогинивания под этим аккаунтом, а не копипастить логин 20 раз по коду.
https://github.com/honeydev/s-blog/blob/master/tests/tests/acceptance/AddPostCept.php#L12Не уверен, что этот тест реально тестирует появление поста. Ведь увидеть текст поста можно например и если форма заполнена неправильно и она выведется для исправления ошибок.
> $I->see('Hi');
Тоже хлипко так как эти буквы вполне могут быть частью другого слова. То есть на любой случайно взятой странице они вполне могут найтись.
> require_once 'mytestslib.php';
В codeception есть тестовые хелперы, надо сделать нормальный класс, а не городить велосипеды. Там можно сделать класс со статическими методами, а также расширять сам объект $I новыми методами.
В тестах на правильное/неправильно заполнение формы много копипасты, надо от нее избавляться, либо через вспомогательные функции, либо через объединение тестов.
https://github.com/honeydev/s-blog/blob/master/sql_cfg.phpтут какие-то костыли. Почему конфиг перемешан с кодом соединения с БД?
Я тебе советую взять и почитать комментарии к нашей задаче про студентов, если ты их не читал.
https://github.com/honeydev/s-blog/tree/master/vendorЭтой папки не должно быть в репозитории. Как и composer.phar и
https://github.com/honeydev/s-blog/blob/master/composer-setup.phphttps://github.com/honeydev/s-blog/blob/master/sblog.sql#L66> ENGINE=MyISAM
Почитай самостоятельно про отличия MyISAM и InnoDB
> userid
Как тут выделить отдельные слова если все написано слитно?
> picuri
Слишком сокращено. Не экономь буквы.
Где внешние ключи в базе?
> CHARSET=latin1;
Почему все только на латиннице? Кирилица не нужна?
Вообще, названия колонок в базе ужасные. К выбору названий надо подходить более тщательно. Почти все имена полей надо переделывать.
> `accesslvl` tinyint(1) NOT NULL DEFAULT '0',
Непонятно что значит эта колонка.
Непонятно чем папка components отличается от app и вообще что такое "компонент"
codeception надо прописать не в require, а в require-dev так как он нужен только разработчику и не нужен для работы сайта.
https://github.com/honeydev/s-blog/blob/master/setup.phpтут дублирование информации из дампа.
https://github.com/honeydev/s-blog/blob/master/public/index.php#L6> 'display_errors', 1);
Это удобнее прописать в php.ini. Или ты на реальном сайте тоже включишь отображение всех ошибок?
https://github.com/honeydev/s-blog/blob/master/app/Services/UserService.phpтут все очень плохо, советую перечитать все комментарии к задаче про студентов, неохота пересказывать. Используй DI, не подставляй переменыне в SQL запросы, почитай про паттерны работы с SQL.
Также, ты не умеешь пользоваться mysqli. После каждого действия с mysqli надо проверять, не произошло ли ошибки, если да, то сообщать о ней, так как автоматически это не проверяется.
SQL запросы у тебя не собраны в отдельные классы, а размазаны по всему коду.
> public function authorizeFromPost
Все смешано в кучу, ты пока не можешь разделить процесс входа на отдельные независимые действия, а пишешь код вперемешку. Тут и чтение данных из POST, и вызов валидации и обработка исключений и чего только нет. Это не MVC.
Вообще, у тебя пока не ООП, а ты просто взял обычный код где все перемешано в кучу и просто добавил сверху слово class. Идея ООП в том, что каждый класс имеет свою зону ответственности.
Ты не делал нашу задачу про Вектор? Стоит сделать, если не делал. И почитать комментарии к задаче про студентов, там много полезного.