«phpClub» — архив тем ("тредов"), посвящённых изучению PHP и веб-технологий.
https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/ Аноним 2018/03/09 16:02:17  №1152248
>>1151740
>>1137298

Надо бы добавить ридми. Прочитай комментарии к задаче, там это написано: https://github.com/codedokode/pasta/blob/master/student-list.md#Публикация-проекта - там же есть ссылка на пример README.

А то открываешь, видишь кучу файлов и даже непонятно, что это. Непонятно, как установить и что надо делать. Пришлось гуглить, разбираться в документации Laravel итд. Очень неудобно.

Желательно убрать код, который не используется. Вот зачем нужен этот файл например? https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/blob/master/tests/Feature/ExampleTest.php

Если ты не используешь тесты, то файлы от них не нужно добавлять.

Также, у тебя там прописана сборка JS-файлов, подключение Vue. Но он нигде не используется. Зачем тогда это добавлять? Из-за этого труднее найти, где именно твой код, а где стандартные заготовки.

Дальше, мне не нравится, что в composer.json прописан "name": "laravel/laravel". Как будто бы это не твое приложение, а официальный репозиторий фреймворка Laravel. По моему, это вводит в заблуждение. Поле name предназначено в первую очередь для добавляемых в репозиторий packagist библиотек, а в твоем случае оно вообще не нужно.

У тебя (вроде бы) сделано так, что таблица студентов недоступна без авторизации, но вообще в задаче она должна выводиться для любых пользователей.

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/blob/master/app/Http/Controllers/Table/TableController.php#L22

Разве это правильно, что в одном случае передается переменная в шаблон, а в другом - нет?

> return view('table', ['orderBy' => $request['orderBy'], 'users' => $users, 'search' => $searchWord]);
> return view('table', ['users' => $users, 'orderBy' => $request['orderBy']]);

Также, не стоило копировать 2 раза эти строки

> $orderBy = $this->validateOrderByParam($request['orderBy']);
> $users = $this->getUsers($request, $orderBy);

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/blob/master/app/Http/Controllers/Table/TableController.php#L47

Получение данных по идее должно быть в модели, а не в контроллере. Ты тут пишешь типичный "толстый контроллер".

Ты используешь в форме регистрации PUT, но ведь браузеры не поддерживают этот метод и там будет использована POST-форма с скрытым полем, а на стороне Ларавел будет сымитировано поступление PUT-запроса. Не переусложнение ли?

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/blob/master/app/Http/Controllers/Profile/ProfileController.php#L22

По условиям задачи требовалось разрешить пробелы, дефисы и апострофы в именах.

> 'birth_year' => ['required', 'string', 'regex:@^199[0-9]|200[0-5]$@iu'],
Люди старше 28 лет - не допускаются?

Далее, я попробовал запустить у себя проект с помощью встроенного в PHP веб-сервера - он показывает страницу "Whoops, looks like something went wrong" (2 раза), но подробности ошибки нигде не выводятся - ни на странице, ни в консоли (выяснилось, что надо создать .env, но я не понимаю, почему по умолчанию подробности ошибки не пишутся в консоль - как это отлаживать?).

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

Также, в задаче была предложена безпарольная регистрация - ты не хочешь сделать такую же? Наверяка в Ларавель это можно настроить.

При сортировке никак не выделена колонка и направление сортировки. Нужно выводить стрелочку или треугольничек или еще как-то показать режим сортировки.

Если при поиске ничего не найдено, то надо бы выводить соответствующую надпись, а не шапку от пустой таблицы.

> protected function validator(array $data)
Имена функций обычно начинаются с глагола, сделайЧтоТо()

> `birth_year` int(11) NOT NULL
В таблице можно было бы использовать тип YEAR.

Насчет этой таблицы https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist3/blob/master/database/migrations/2014_10_12_100000_create_password_resets_table.php - разве токен сброса пароля не должен привязываться к пользователю?

Насчет форм - я вижу, ты используешь бутстрап. Так почему бы не использовать стандартные стили бустрапа для форм? http://getbootstrap.com/docs/3.3/css/#forms

Так, конечно, при использовании Ларавел тут получается очень мало твоего кода (так-то это хорошо, но у нас ведь задача научиться). На мощном фреймворке лучше было бы делать задачу вроде TestHub, где все сложнее. Ну или попробовать сделать безпарольную авторизацию, как описано в задаче.
Ответы: >>1152276
https://github.com/moabit/student-list/ Аноним 2018/01/18 04:55:37  №1121562
>>1117075
>>1117974
>>1118558

Я там обновил комментарии к задаче, посмотри, может что нового прочтешь.

> https://github.com/moabit/student-list/blob/master/composer.json#L5
> "fzaninotto/faker":"^1.7.1"

Это лучше было бы прописать в require-dev, то есть в зависимости для разработки. На продакшене ведь faker не требуется.

> https://github.com/moabit/student-list/blob/master/config.json
Этот файл используется в git и тут есть проблема: если разработчик склонирует твой проект и пропишет какие-то настройки, то он может закоммитить назад в проект измененный файл. Решение я описал в уроке: https://github.com/codedokode/pasta/blob/master/student-list.md#%D0%A1%D0%BA%D1%80%D1%8B%D1%82%D0%B8%D0%B5-%D1%84%D0%B0%D0%B9%D0%BB%D0%BE%D0%B2-%D0%B8%D0%B7-%D1%80%D0%B5%D0%BF%D0%BE%D0%B7%D0%B8%D1%82%D0%BE%D1%80%D0%B8%D1%8F

> https://github.com/moabit/student-list/blob/master/public/index.php
> ini_set('display_errors', 1);
Это лучше прописывать в php.ini, а то не хочется, чтобы на продакшене ошибки выводились. Можно конечно прописать настройку в конфиге, но непонятно зачем дублировать функционал php.ini.

> https://github.com/moabit/student-list/blob/master/app/container.php#L20
> SET NAMES utf8mb4,
Вроде в PDO сделали опцию charset в конструкторе или в DSN.

https://github.com/moabit/student-list/blob/master/app/container.php#L35
> $container['pager'] = function ($c) {
Это не имеет смысла, так как у Pager есть конструктор. Также, ты по идее должен не переиспользовать один экземпляр Pager, а создавать каждый раз новый.

Также, мне кажется, в контейнере не стоит хранить классы с состоянием вроде Authorisation, а стоит создавать новую копию при каждом обращении. Представь, что мы обрабатываем 2 HTTP-запроса подряд. Первый запрос изменит состояние авторизации, и оно может повлиять на выполнение второго запроса.

> throw new RouterException('Неправильный путь');
надо назвать класс лучше, так как этот класс генерирует 404 и должен использоваться только в таких ситуациях, когда надо отдать 404. Например, RouteNotFoundException или Http404Exception.

> https://github.com/moabit/student-list/blob/master/app/Controllers/Controller.php#L18
> protected $view;
Эту переменную наверно лучше назвать twig

https://github.com/moabit/student-list/blob/master/app/Controllers/Controller.php#L37
Может, тут лучше было сделать абстрактную функцию index?

https://github.com/moabit/student-list/blob/master/app/Controllers/Controller.php#L42
> protected function getUserData()
Не очень понятно назначение метода. Он точно должен быть в каждом контроллере?

https://github.com/moabit/student-list/blob/master/app/Controllers/ProfileController.php
здесь наверно лучше было бы сделать все же общую функцию для редактирования/регистрации, чтобы не передавать данные через поля. Такая передача данных затрудняет анализ логики кода.

> if (Util::checkCSRFToken() == false) {
Можно писать if (!Util::checkCSRFToken()), почитай про булевы значения.

https://github.com/moabit/student-list/blob/master/app/Controllers/ProfileController.php#L60
> $this->c['authorisation']->signIn($token);
> $student->setToken($token);
вот тут есть тонкий момент. Что, если мы сгенерируем токен, поставим куку, а потом скрипт упадет, не записав данные в БД. Пользователь останется с кривым токеном. Это не проблема? Хотя, если сделать наоборот, может получиться, что мы создадим пользователя в БД, а куку не выдадим - будет еще хуже.

https://github.com/moabit/student-list/blob/master/app/Controllers/ProfileController.php#L97
> $student->setName(ucfirst(trim(strval($_POST['name']))));
ucfirst не работает: https://github.com/codedokode/pasta/blob/master/php/strings-utf8.md

https://github.com/moabit/student-list/blob/master/app/Controllers/ProfileController.php#L93
> $student = new Student;
> if ($this->user) {
> $student->setID($this->user->getID());
> }
При обновлении данных лучше сделать так: загрузить студента из БД и затем обновить ему поля из POST. А то в твоем случае, если добавить какие-то новые поля, которые не редактируются через форму, но хранятся в БД и есть в объекте, то они могут потеряться при редактировании.

> private function editUser($token): void
Для $token можно поставить тайп-хинт

https://github.com/moabit/student-list/blob/master/app/Database/StudentDataGateway.php#L36
> if ($search != null) {
> $search = trim($search);
Если передать строку из одних пробелов, найдутся все студенты.

https://github.com/moabit/student-list/blob/master/app/Database/StudentDataGateway.php#L54
> @return array
> public function getStudents
лучше писать @return Student[] - это несет больше информации - смотри мануал https://docs.phpdoc.org/guides/types.html

> public function getStudents($orderField, $direction, $limit, $offset, $search = null):
Тайп-хинтов бы сюда завезти...

> throw new \PDOException('Неверный параметр сортировки');
В сообщение можно добавить значение ошибочных параметров для упрощения отладки.

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

https://github.com/moabit/student-list/blob/master/app/Helpers/Authorisation.php#L75
> return $this->isAuth;
тут не return null должно быть?

https://github.com/moabit/student-list/blob/master/app/Helpers/ErrorHandler.php#L33
> header("HTTP/1.0 404 Not Found");
> echo "Страница с таким адресом не существует";
без указания кодировки кириллица может не отобразиться. Добавь либо Content-Type либо тег meta charset. Также, на мобильных надпись будет выводиться очень мелко, можно добавить meta viewport.

https://github.com/moabit/student-list/blob/master/app/Helpers/Util.php#L27
> throw new ConfigException('Ошибка в файле конфигурации');
Стоит тут добавить текст JSON ошибки.

https://github.com/moabit/student-list/blob/master/app/Helpers/Util.php#L58
> $_COOKIE['CSRFToken'] != $_POST['CSRFToken']
Тут надо использовать строгое сравнение. Также, надо проверить случай, когда оба значения пусты.

https://github.com/moabit/student-list/blob/master/app/Validators/StudentValidator.php
Тайп-хинтов бы побольше.

https://github.com/moabit/student-list/blob/master/app/addStudents.php
Это лучше было наверно сделать в виде cli скрипта.

> $student->setSurname($faker->lastName.'а');
Faker не умеет в женские фамилии? Не хочешь исправить этот баг и законтрибутить исправление в Faker на благо всех? Сначала, конечно, надо повнимательнее изучить Faker, может там уже есть решение.

https://github.com/moabit/student-list/tree/master/app/views/templates
Может, стоит эту папку вынести на верхний уровень?

https://github.com/moabit/student-list/blob/master/app/views/templates/studentlist.twig
> {% if students is not empty %}
> много строк
> {% else %}
> <h4 class="text-center m-4 text-muted">По вашему запросу ничего не найдено</h4>
> {% endif %}
Лучше может быть короткий блок ставить сверху. Для читабельности.

> >{{ pager.search|e }}
В твиге автоэкранирование.

> {% elseif pager.notify=='registered' %}
Это как-то неправильно, что pager используется для хранения нотификаций, согласись? Это не его задача.

> <a href="?{{ pager.getSortingLink('name') }}"
Лучше возвращать ссылку уже с вопросом, а не собирать ее так по кускам.

Для twig надо бы включать strict_variables. Иначе он не скажет об отсутствующей переменной.

Вообще, код выглядит очень неплохо. Ты ведь наверно не начинающий? Может тогда автоматические тесты сделаешь? Урок есть: https://gist.github.com/codedokode/a455bde7d0748c0a351a
Ответы: >>1122873 >>1131535
Аноним 2018/01/17 02:01:15  №1121055
>>1120998

Метод зависит от вида формы.

GET - для форм, не изменяющих состояние сервера, с небольшим объемом данных, и чтобы можно было поделиться/сохранить ссылку на результат (например: поиск, сортировка, переход к странице)

POST - для форм, меняющих состояние сервера, для форм с закачкой файлов, для больших объемов данных. Ссылкой поделиться нельзя, при обновлении страницы выскочит предупреждение.
Ответы: >>1121122
https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2 Аноним 2018/01/11 02:11:46  №1118527
>>1113346

Желательно выделить публичную папку, а не выкладывать весь проект в нее.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/composer.json
Имена неймспейсов желательно писать с большой буквы, хотя в PSR-4, такого требования вроде нет.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/dump.sql
Ключевые слова языка SQL принято писать заглавными: http://www.sqlstyle.guide/ru/#зарезервированные-слова

id обычно идет первым в списке полей.

> passwordhash
> second_name
Слова надо разделять единообразно.

> the_group
Лучше просто добавить кавычки `group` (это в MySQL так, в стандарте SQL используется "group", ссылку на стандарт дать не могу, так как он стоит денег: http://modern-sql.com/standard , в общем доступе есть черновики стандарта 2011).

> token text null
Тут нужно бы добавить комменатрий и указать тип поля поменьше.

> $routeWithoutGETParam = $this->delGETParametersFromRoute($routes);
Используй лучше parse_url

> 'studlist\\controllers\\'.$routeWithoutGETParam[1].'Controller
На линуксе имена файлов чувствительны к регистру.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/controllers/TableController.php#L14

> private $title = '';
> private $db;
> private $searchData;
А зачем сохранять эти данные в поля класса? Не лучше ли просто сделать их локальными переменными?

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/Student/Student.php
> public $parameters = array('login' => '', 'email' => ''

Это я называю "массиво-ориентированное программирование". Ты тут используешь массив, хотя и пытаешься сделать видимость использования ООП. Почему бы не сделать объект со свойствами login, email и тд?

> public function setStudentParametersFromForm()
Модель в MVC не должна обращаться к параметрам HTTP запроса в $_POST. Также, посмотри, как странно выглядит код в контроллере:

> $this->student->setStudentParametersFromForm();
В метод ничего не передается, он откуда-то сам берет данные.

> $this->parameters[$name] = trim(htmlspecialchars($_POST[$name], ENT_QUOTES));
Зачем тут использован htmlspecialchars? Его используют при вставке текстовых данных в HTML код. Но здесь, в модели, мы еще не знаем, как будут использованы данные, будут ли они когда-нибудь вставляться в HTML-код. Зачем тогда использовать htmlspecialchars?

Плюс, у тебя нет никаких проверок, какие поля ты берешь из $_POST. В этой задаче это не проблема, но в другой ситуации это позволяет пользователю модифицировать любые поля, установить себе признак администратора, например.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/UsersTableGateway/UsersTableGateway.php#L42
> return $stmt->fetch(\PDO::FETCH_ASSOC)["passwordhash"];
Есть метод в PDOSTatement для получения единственного значения.

> if ($stmt->fetch(\PDO::FETCH_ASSOC)["COUNT(*)"])
То же самое.

> public function getStudentsForTable($orderByLimitOffset)
Здесь передается массив. Но нигде не описано, что это за массив, какие в нем могут быть аргументы. Лучше либо сделать отдельные аргументы, либо передавать объект.

> $stmt = $this->db->prepare("SELECT name, second_name, the_group, points
> ORDER BY $orderBy
Здесь значения вставляются напрямую в запрос без проверки - возможна SQL инъекциия.

> return $stmt->fetchAll(\PDO::FETCH_CLASS, 'studlist\model\Student\Student');
Эта команда не поместит значения из БД в массив $parameters внутри объекта.

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/UsersTableGateway/UsersTableGateway.php#L107
Что это за странный способ обработки исключений? С каких пор модель делает куда-то редиректы?

> ПОЧЕМУ ЕСЛИ Я ДЕЛАЮ БИНД orderBy ТО сортировка не работает?
Потому, что значения плейсхолдеров вставляются в запрос в кавычках, вроде 'name', и получается фиксированная строка (по которой идет сортирвка), а не название колонки.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/Verification/Verification.php#L53
> public function VerificationFormsStudent
Имена функций принято начинать с глагола, сделайЧтоТо().

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/Verification/Verification.php#L138
> static public function verificationLogined(UsersTableGateway $db): bool
Лучше внедрить объект $db в валидатор с помощью DI, через конструктор.

Также, непонятно, почему валидатор сам лезет в куки. Модель в MVC не должна напрямую обращаться к кукам. Перечитай внимательно урок по MVC.

И непонятно, зачем ты там обрабатываешь значения кук с помощью htmlspecialchars. Это ни от чего не защищает.

Я бы советовал тебе сделать задачку про экранирование отсюда https://github.com/codedokode/pasta/blob/master/soft/web-server.md#Экранирование чтобы лучше разобраться с разными видами экранирования данных.

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/Pager.php
Здесь, по моему, не очень удачно спроектирован обьъект. Там есть публичное поле, которое заполняет метод setPager(). Но, когда у тебя есть этот объект в переменной, ты не знаешь, был ли ранее вызван метод или нет, и не понимаешь, есть ли нужные данные в поле или нет. Это не позволяет писать надежный код.

Вместо поля лучше сделать метод getPageNumbers(), который гарантирванно возвращает нужные значения. Сделать поля приватными и использовать инкапсуляцию.

Что-то у тебя слабовато знание ООП. Ты не решал задачу про Гостиницу или про Продюсерское Агентство? Они есть в треде. Я бы советовал отвлечься и решить их.

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/StudentsBox.php
Не очень понятно, зачем вообще нужен этот класс? Что моделирует объект этого класса, какую сущность?

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/model/ViewRender.php
Тут явное нарушение принципа единой ответственности (каждый класс решает свою задачу, имеет свою зону ответственности) - класс отвечает и за вывод view, и за расчет пагинации, хотя это никак не связанные между собой задачи.

Не очень понятно, зачем здесь сделано разделение страницы на "заголовок" и "тело".

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/controllers/TableController.php#L98
Непонятно, почему контроллер вывода списка студентов отвечает за разлогинивание или залогинивание.

> if ($_GET['orderBy'] != ('points' || 'the_group' || 'name' || 'second_name')) {
Это так не работает. Оператор || - это оператор, который возвращает true или false. http://php.net/manual/ru/language.operators.logical.php

Получается в итоге выражение вроде

if ($_GET[...] == true)

И это явно не то, что нужно. Тебе наверно подойдет in_array.

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/controllers/ErrorController.php
для страницы ошибки нужно выдавать отличный от 200 код HTTP: https://ru.wikipedia.org/wiki/%D0%A1%D0%BF%D0%B8%D1%81%D0%BE%D0%BA_%D0%BA%D0%BE%D0%B4%D0%BE%D0%B2_%D1%81%D0%BE%D1%81%D1%82%D0%BE%D1%8F%D0%BD%D0%B8%D1%8F_HTTP

Если ты не знаком с HTTP, прочти мой урок по нему: https://github.com/codedokode/pasta/blob/master/network/http.md

Для залогинивания/разлогинивания пользователей желательно сделать отдельный класс.

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/view/login.phtml
Не надо использовать echo, лучше использовать <?= ?>

https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/view/profile.phtml
не вижу здесь защиту от XSS.

эти 2 файла очень похожи:

- https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/view/profile.phtml
- https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/view/reg.phtml

не лучше ли сделать из них один?

> https://github.com/dsgaljkeguhodgiosetuhsegjposguh/studlist2/blob/master/view/table.phtml
Для формирования ссылок для сортировки или пагинации лучше сделать специальную функцию, а не делать это прямо в шаблоне. Также, символ & в HTML коде надо вписывать как & amp;

Ответы: >>1118558
Аноним 2018/01/06 06:36:18  №1116330
>>1116260

Считаешь, сколько всего в БД студентов, делишь на допустим 20 и получаешь число страниц. Выводишь список страниц. Номер страницы можно передавать в параметре page, то есть ссылки могут выглядеть так:

/students.php (1-я страница)
/students.php?page=2 (2-я)
/students.php?page=3 (3-я)

итд.

Не забудь, что если выбрана сортировка или поиск, то их параметры надо тоже добавлять в ссылку. Если идет поиск, то для расчета числа страниц нужно считать только соответствующих условиям поиска студентов.

Для выборки части результата в MySQL есть нестандартная (ее нет в стандарте SQL) опция LIMIT.

>>1116219

Есть еще интересный вариант - разбивать по границе слова (\b): https://regex101.com/r/bN3dsN/2 (я добавил флаг u для поддержки юникода).

Также, можно использовать \W (не-слово).

Не знаю, подойдет ли.

>>1116216

Надо писать [^а-яё] так как "ё" не входит в диапазон "а-я", а идет отдельно.

>>1116122

Это веб-сервер, отвечающий на HTTP запросы. Подробнее: https://github.com/codedokode/pasta/blob/master/network/http.md

Ответы: >>1116463
Аноним 2017/12/27 03:14:55  №1112991
>>1111682

count(descendant::...) = 0

>>1112485

Если ты не используешь анонимные функции, это не значит, что они не нужны. Я их использую, например. Например, для сортировки по произвольному критерию, вместе с array_map/array_filter, и тд. Добавили их в 5.4.

Анонимные классы - насколько я знаю, типичный пример это обработчики событий в Ява, наверно тут они для той же цели.

>>1112491

Session fixation работает тогда, когда посторонний может как-то задать id сессии. Например, если он передается через GET-параметр, злоумышленник дает пользователю ссылку example.com?sid=12345678, пользователь залогинивается (и это записывается в сессию 12345678), после чего злоумышленник сам заходит по такой же ссылке и оказывается тоже залогинен (так как у него тот же id сессии, в которой стоит признако залогиненности).

Потому не надо передавать id сессии через что-то, кроме кук.

>>1112490

Еще есть вариант просто не использовать сессии вообще. Не так они и нужны.

>>1112483

Рассмотри еще вариант не использовать сессии вообще.

Пароль не надо хранить открытым текстом, храни соленый хеш от него. Иначе злоумышленник, получив доступ к серверу или дискам от него, или бекапам, получает готовые пароли.

> каждые 20 минут генерация нового ид сессии
Непонятно, зачем. Ты приводил ниже пример, но в браузере куки общие для всех вкладок.
Аноним 2017/12/16 18:54:26  №1108431
>>1107288

У тебя наверно не используются виртуальные хосты, а только один хост в сервере прописан. Тогда можно так и оставить.

>>1107265
>>1107266

Такой интересный вопрос, и никто в треде не комментирует? Что же вы так.

> 1. заканчивать названия объектов на -er - это анти ооп http://www.yegor256.com/2015/03/09/objects-end-with-er.html

По моему опыту, да, часто объекты делятся на "сущности", у которых есть свойства, и "сервисы", у которых нет свойств и которые есть только в 1 экземпляре. Он предлагает ликвидировать "сервисы" и их код перенести в "сущности". Но это не всегда удобно:

- если сервис использовал DI, то теперь мы должны передавать зависимости при создании сущности, хотя они, может быть, ей редко когда нужны и выглядят "лишними". То есть в объект User мы должны передавать PDO, и еще какие-то зависимости.
- если сервис создает сущности (StudentGateway->findStudentById(..)), то как этот код перенести в Student?
- мы должны смешивать функционал в одном классе и менять паттерны в угоды этой идее. Например, в соответствии с его идеями, мы должны отказаться от Data Mapper и заменить его на Active Record, которая имеет недостатки (много функционала собрано в одном классе).
- на практике, с одной сущностью часто можно делать очень много вещей. Возьмем Пост в блоге: опубликовать, прокомментировать, лайкнуть, проверить на правильность, сохранить в БД, загрузить из БД, поделиться ссылкой на Пост по почте. Ну или возьмите Товар в интернет-магазине. Или Пользователя в соцсети. Там просто тонна функционала и класс станет огромным, и будет напоминать God object.

Если подумать над решением указанных проблем, "сервисы" как раз помогают их решить. Вот также статья про Service Layer: http://design-pattern.ru/patterns/service-layer.html

Возможно, что это не "чистый ООП". А это где-то описано, каким должен быть "чистый ООП"? Я не знаю.

А какие вы видите еще варианты решения? Попробуйте взять реалистичный код (посложнее чем сортировка яблок) и избавиться там от сервисов.

По второму вопросу.

> Isn't it already clear that a single-argument constructor of class CSV expects the name of a file with comma-separated values? I would rename it to file
fileName как раз лучше, так как file у меня ассоциируется с объектом вроде SplFileObject.

> 3. пустые строки - это тоже code smell, т.к. они означают, что метод не соблюдает принцип единой обязанности.
Мне все же больше нравится вариант с пустыми строками. Не надо преумножать сущности (методы) без необходимости, если метод не слишком большой, то хватит и перевода строки. Это мое субъективное мнение.

> и если он прав, то не слишком ли это высокие материи, чтобы задумываться о них на джун-уровне?
Почитать такую статью и попробовать составить свое мнение было бы полезно.
Ответы: >>1108547 >>1108693
Аноним 2017/11/26 03:11:28  №1097912
В задаче про студентов при сортировке списка тоже должны быть ЧПУ? Я пока только с ссылками такого вида запилил: example.com/?field=surname&direction=desc
Ответы: >>1097943
Аноним 2017/11/24 21:50:52  №1097370
>>1094744

Значит шторм просто не поддерживает эмуляцию терминала с управляющими кодами.

>>1094692

> Так у меня много мест где куки идут ниже по странице и хедеров тоже есть
А не должно быть. Ты сначала выполняешь всю логику на PHP и подготавливаешь данные, а только потом выводишь страницу.

> Щас исправил, только ордер бай по столбцам нельзя делать через prepare, иначе он выдаёт по айдишникам на любой запрос,
Потому что при подстановке через плейсхолдер получается ORDER BY 'name', то есть сортировка не по значению поля, а по константе. Так что да, ты правильно сделал.

Но проверка должна быть в классе TableGateway, так как иначе где гарантия, что в нее поступят проверенные данные? И как увидеть, что они проверяются? Проверять их логичнее прямо перед вставкой в запрос, а не где-то в другом месте кода.

> Я когда захожу из браузера меня логинит по кукам, которые задавались при регистрации, это ты имел ввиду?
Нет, в данном случае, тебя должно залогинить без регистрациии и без наличия кук. Под каким-то существующим аккаунтом.

>>1094685

> "<a href='script.php?{$link}'>
Ты при вставке URL в HTML-код, в атрибут href, не делаешь экранирование. Ну например, если в ссылке есть символ & то он дложен вставляться как & amp ;

Ты проверял, эта ссылка корректно воспроизводит все символы из примеров к задаче?

Ну и второй скрипт писать не требовалось, можно было сделать, чтобы ссылка указывала на тот же самый скрипт (index.php). Может это я, конечно, плохо сформулировал задачу.
Ответы: >>1097468
Аноним 2017/11/17 19:24:33  №1093586
>>1093391

Тут есть одна проблема. Table Data Gateway - это класс, в котором хранятся методы для взаимодействия с БД. Функция "checkData(переменные с форм)" - в этот паттерн никак не вписывается.

Надо либо переименовать класс, либо перенести функцию в класс-валидатор (валидация - это и есть проверка данных).

Также, название Table Data Gateway намекает, что класс может работать с любой таблицей. Но этот класс заточен только под работу с таблицей студентов, и лучше это отразить в названии.

Что касается авторизации, то ее, конечно, тоже в Table Data Gateway быть не должно. Ее лучше поместить в отдельный класс, отвечающий за это.

В ООП есть принцип Single Responsibility - один класс отвечает только за одну какую-то зону ответственности. Взаимодействие с БД и авторизация - это очень разные вещи и их нужно разнести в отдельные классы. То, что это разные вещи, легко увидеть даже по тому, что они взаимодействуют с разными сущностями - TDG работает с базой данных, а авторизация с куками (а когда классу авторизации нужно получить что-то из базы, он не лезет в нее напрямую, а обращается к TDG).

Также, в твоем описании немного не хватает подробностей:

> checkData(переменные с форм)
Не написано, чем является (какой тип данных имеет) аргумент "переменные с форм". Напиши это.

> функция getStudentInfo(без аргументов)
> - возвращает информацию о студенте
Не указан тип возвращаемых данных

> функция getStudentPage(общее кол-во студентов, сколько выводить на странице, сортировка(необязательный пар-р)):
Ты указываешь, сколько надо взять студентов. Но непонятно, как получить студентов для второй, третьей и тд. страниц. Нужно еще добавить параметр offset (сколько студентов надо пропустить с начала списка).

И еще несколько неудачно выбраны названия некоторых функций. Хорошо выбранные названия делают код намного понятнее, и тут есть сложившиеся традиции, как называть те или иные функции:

- searchPage - "искать страницу" или "страница поиска". Имена функций начинаются с глагола, "сделайЭто", и лучше функцию назвать "найти студентов" - это ведь ее задача. В MVC модель не должна беспокоиться о том, как и где будут использоваться данные из нее, за это отвечает контроллер. Когда ты пишешь в модели название страницы, где будет использоваться функция, ты как бы нарушаешь это разделение ответственности.
- getStudentPage - "получить страницу студентов" - лучше просто "получить студентов"
- checkData - "проверить данные" - непонятно, какие данные. Лучше назвать "проверить данные студента", или, выкинув бесполезное слово "данные", "проверить студента". Вместо check можно использовать validate, которое значит "проверить на соответствие правилам" и используется как раз втаких случаях
- "checkAuth" - "проверить авторизацию" - такие функции, которые возвращают да/нет, принято называть со слов is/has/can/does, например: isAdmin, canDeletePage, hasPremuimStatus. Тут лучше использовать название "является ли авторизованным" - isAuthorised, isUserAuthorised
- "getStudentInfo" - "получить информацию о студенте" - "информация" - это бессмысленное слово, которое не говорит ничего нового и которое лучше избегать. Лучше написать "получить залогиненного студента" "getLoggedInStudent", "получить текущего пользователя" - getCurrentUser, "получить авторизованного пользователя" - getAuthorisedUser
- функция enterSite - "зайти на сайт" - это функция с результатом да/нет, нужно ее назвать как принято в таких случаях
- setCookie - "поставить куку". Неудачное название, лучше назвать ее "залогинить пользователя", "авторизовать пользователя". Не конкретизируя, как именно это делается. НАм (когда мы вызываем функцию) ведь важно, чтобы он в итоге был залогинен, а не как это будет сделано. То же самое с delCookie.

И еще. Чтобы представить значения да/нет, вместо цифр лучше использовать специальный булев тип: http://php.net/manual/ru/language.types.boolean.php . Операторы сравнения вроде ==, <, > как раз возвращают значения булева типа.

Что касается авторизации, у тебя там есть 2 функции для проверки залогиненности: проверить наличие кук и проверить содержимое кук по базе. Но нам ведь интересно только, залогинен пользователь или нет, нам не хочется самим где-то брать его куки и куда-то передавать. Не логичнее ли сделать одну функцию, которая сделает все необходимые проверки и окончательно отвечает на вопрос, залогинен он или нет?

Как видишь, в итоге модель предоставляет что-то вроде набора API (набора класссов и функций, которые можно вызвать, чтобы выполнить какое-то действие с приложением). Важно, чтобы оно было удачно спроектировано, удачно названы функции - код тогда будет читать намного проще.

И, наконец, еще один момент. У тебя некоторые функции принимают или возвращают данные о студенте? В каком они могут быть виде? Могут быть в виде массива, а могут быть в виде объекта (который хранит информацию об одном студенте). Подумай и напиши, какие плюсы и минусы есть у каждого из этих подходов и на основании этого выбери, что лучше подойдет для твоей модели.

После того, как ты исправишь проблемы с моделью, нужно ее написать (используя в том числе старый код), пока без контроллеров ии вью. И, чтобы показать, что твоя модель работает, сделай несколько тестовых скриптов:

1) скрипт, который вставляет в БД пользователя с прописанными в скрипте данными
2) скрипт, который при заходе из браузера логинит под определенным аккаунтом
3) скрипт, который при заходе из браузера пишет, под каким аккаунтом залогинен пользователь

После этого скинь ссылку на код, но не жди меня, а двигайся дальше. Можно переходить к написанию контроллеров и вью.