«phpClub» — архив тем ("тредов"), посвящённых изучению PHP и веб-технологий.
Аноним 2018/12/09 08:08:37  №1308429 1
ОП, глянь "Гостиницу" мою, если будет время:
http://sandbox.onlinephpfunctions.com/code/bf265b1dcfaab38559a459812aa03f0c5583fc7b

Также есть вопрос по оформлению кода, в PSR-1, PSR-2 ответа не нашел, PHPStorm на автомате не форматирует это. Как правильно разбивать построчно различные конструкции. Вот примеры у меня в коде - 55-60 строки (сокращенный if), 188-192 строки (длинное условие в if-е), 268-274 строки (массив).
Ответы: >>1308859
Аноним 2018/12/10 00:49:34  №1308859 2
>>1291781

Надо сделать сущность "рассылка" и связь многие-ко-многим между рассылками и пользователями, чтобы было записано, кому какая рассылка отправлялась. Сложность в том, что если вставлять запись до отправки письма, то пользователь при ошибке отправки второй раз не будет выбран. А если после - то при ошибке вставки записи пользователь может получить второе письмо.


>>1308429

> public function addRooms(array $roomsForAdding)
Это неудобная функция, так как непонятно, какого формата массив в нее передается. Логичнее было принимать сразу массив объектов-комнат. Это гораздо гибче, так как в таком случае можно как угодно настроить эти комнаты, и даже передать вместо комнаты ее наследника (правда, не очень представляю, зачем). А у тебя - нельзя.

Или хотя бы сделать функцию addRoom() с человекопонятными параметрами.

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

> echo "Добро пожаловать в гостиницу '$name'";
Это для отладки? в реальном коде такого быть не должно. Представь, ты хочешь на сайте вывести информацию о комнатах, создаешь объект, а он пользователю на страницу выводит надпись "добро пожаловать", причем в самом верху страницы, еще до шапки, и в неправильной кодировке (так как метатег, задающий кодировку, еще не передан).

Гостиница не должна сама ничего выводить. Она только принимает и возвращает значения.

> public function checkGuests(array $guestsList, string $entryDate, string $departureDate)
> string $entryDate
Почему не DateTimeInterface? У нас уже есть объект для представления времени. Это избавляет нас от необходимости проверять правильность строки, например.

> public function checkGuests(array $guestsList, string $entryDate, string $departureDate)
Стоит добавить проверку, что вторая дата больше первой. Это позволит быстро обнаружить ошибку программиста. А так - ошибка останется не замеченной, просто не получится заселить гостей, как будто номеров нету. Это собьет всех с толку и затруднит обнаружение причины проблемы.

> // сортировка пузырьком по (цене/кол-во гостей) по возрастанию
usort тут не подходит? Надо как минимум это выносить в отдельную функцию, это же тяжело читать, там такие многоэтажные выражения.

> $guestsForRegestration = array_slice($guestsList, 0, $roomForRegestration->getCapacity());
> $guestsList = array_slice($guestsList, $roomForRegestration->getCapacity());

Есть array_splice для этого.

В общем, checkGuests слишком большая для понимания, надо ее уменьшить. И кстати, у тебя есть 2 пути - регистрация всех в одном номере и регистрация в нескольких номерах - но ведь первое это частный случай второго и можно использовать для них один и тот же код.

> public function printFreeRooms
Гостиница ничего не выводит. Она должна просто вернуть данные. Выводом занимается внешний код. Разделение ответственности.

> public function printGuestHistory(string $name)
Стоило бы Гостя тоже сделать объектом, можно даже без полей. Ну и интуиция подсказывает, что Гостиница захочет знать, кто вместе с кем приезжает, потому может понадобиться даже ГруппаГостей. Но делать ее пока не надо, раз это не просят.

> if (in_array($name, $regestration->getGuests())) {
Лучше бы сравнивать не имена, а сами объекты. Объект обладает идентичностью, он уникален, и подходит для сравнения.

> for ($fromTheDate; $fromTheDate <= $toTheDate; $fromTheDate->add(new DateInterval('P1D'))) {
> $dailyIncome = 0;
> foreach($this->regestrations as $regestration) {
А нельзя ли тут как-то оптимизироваться за счет, например, сортировки броней по дате? Или, например, первым проходом отсеять брони, попавшие в интервал, а вторым - пройтись по ним и посчитать массив сумм по дням. А то у тебя тут сложность O(days x regestrationsCount) получается.

> if (($entryDate >= $regestration->getEntryDate() && $departureDate <= $regestration->getDepartureDate()) ||

А можно еще сделать if ($regestration->includes($entyrDate)) или даже $regestration->intersects($entry, $departure). Это повысит читабельность. А то ты относишься к объектам как к массивам, которые ничего не умеют кроме хранения данных и ничего сами сделать не способны.

Также, здесь можно было ради читабельности сократить $regestration до $r.

> array_splice($freeRooms, $key, 1);
Плохая новость: у array_splice и у array_search сложность O(N), а у array_key_exists/unset() всего лишь O(1), то есть гораздо быстрее. Массивы заточены на поиск по ключу, а не значению. Можно бы оптимизировать, заменив splice на unset. Еще один вариант - использовать SplObjectStorage, который умеет хранить коллекцию объектов как ключи массива и искать или удалять их за O(1).

Если у нас 100 номеров и 10 000 броней, то получается порядка 100 x 10 000 шагов, а с оптимизацией - в 100 раз меньше.

Это конечно задача на ООП, но почему бы заодно не поучиться оптимизации?

> public function __construct(Room $room, DateTimeImmutable $entryDate, DateTimeImmutable $departureDate, array $guestsList) {

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

И еще, для проверки можно бы использовать юнит-тесты. Вот я тут подробно про них писал: https://phpclub.tech/pr/res/1281608.html#1303707

И вот что у анона получилось: https://ideone.com/Kj2GBG

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

> Как правильно разбивать построчно различные конструкции. Вот примеры у меня в коде - 55-60 строки (сокращенный if), 188-192 строки (длинное условие в if-е), 268-274 строки (массив).

А точно ли PSR не говорит?

> 5. Управляющие конструкции
> Перед закрывающими круглыми скобками НЕ ДОЛЖНО быть пробелов.
> Между закрывающей круглой скобкой и открывающей фигурной скобкой ДОЛЖЕН быть один пробел.

Тут кое-что написано. У тебя явно не выполняется требование про отсутствие пробелов перед круглой скобкой.

А вообще, если что-то не определено в PSR, и есть несколько вариантов, то лучше определить это в документации к проекту или в внутрикорпоративном стандарте. Можно также посмотреть, как это сделано в Симфони и сделать так же: https://github.com/symfony/symfony

Также, можно почитать https://github.com/php-fig/fig-standards и поднять вопрос в их mailing list. Например, предложить свой вариант стандарта. Вот здесь ты можешь почитать, как неспешно идет обсуждение новых стандартов: https://groups.google.com/forum/#!forum/php-fig