All streams
Search
Write a publication
Pull to refresh
1
0
Паша Макаров @Metus

PHP/Rails-программист

Send message

Почему-то многие считают, что модель — это классы в папочке models.
Модель — это не класс и не объект. M в MVC — это самое абстрактное определение модели из существующих.
Модель — это источники данных и правила их обработки, а работа приложения — моделирование.


Объект, хранящий космический корабль — часть модели. Расчёт лазеров в зависимости от цели — тоже часть модели. Какой-то там сервис для обработки объектов — тоже часть модели.

Нет, я предлагаю вынести метод isCurrentUser из модели. При желании, расширить Auth и вынести данную логику туда.


Либо можно вообще сделать трейт для контроллеров. Потому как решать, авторизован пользователь или нет должны именно они.

Конечно нормально. В приложении всегда один уровень зависит от другого.


В тему цитат приведу первые 4 строчки философии python:


  • Красивое лучше, чем уродливое.
  • Явное лучше, чем неявное.
  • Простое лучше, чем сложное.
  • Сложное лучше, чем запутанное.

Метод isCurrentUser неявный и запутанный.


Неявный потому как данные зависимости, простите за тавтологию, не являются явными.


Запутанный из-за того, что зависимость кольцевая: модель пользователя (модель бизнес-логики) зависит, от сервиса более высокого уровня (который её и использует).

Какая уж тут теоретика?


Конкретный случай. Разработчику нужно вызвать код


$user->isCurrentUser()

Он знает, что этот метод корректно работает в контроллере.
Но сейчас ему нужно вызвать этот код в консоли, а завтра в контексте API-контроллера.


Будет ли это работать без предварительных приготовлений где-то в сервис-провайдере? Нет.


Знает ли об этом разработчик? Нет. Всё что у него есть — это метод, который каким-то образом (видимо, по контексту вызова) определяет, авторизован пользователь, или нет. Как он определяет — разработчик не знает. Он и не должен знать — инкапсуляция.


К вопросу о простоте, он мог бы вызвать:


Auth::user() && Auth::user()->id == $user->id

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


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


$headerUserToken == $user->token

Но у него есть этот волшебный метод isCurrentUser, который скрывает всю информацию об использовании сервиса.


  • Использует ли он какие-то сервисы — разработчик не знает.
  • Будет ли работать он, если пользователю пришёл HTTP-заголовок secret_user_token — он не знает.
  • Будет ли работать он, если второй и третий аргументы командной строки являются логином и паролем — он не знает.

Он и не должен ничего этого знать.


Почему Вы всё время пытаетесь рассказать как работают сервис-провайдеры в ларавеле? Мои претензии не к ним, а непосредственно к модели User.

Сервис авторизации стандартный, но использование его нестандартное.


Есть объект $user.


Человек видит в нём метод isCurrentUser. Без копания в коде (а в идеале нужно смотреть только на сигнатуру метода) не понятно как заставить его работать, и будет ли он работать вообще без дополнительных манипуляций.


Что делать разработчику?


  • Будет ли метод работать сразу, потому как мы обрабатываем http-запрос?
  • Нужно ли вызывать Auth::login снаружи, т.к. http-запрос отсутствует?
  • Нужно ли делать ещё что-то?

Всё это неясно. Именно в этом проблема таких неявных и при этом жёстких зависимостей.


Объект должен быть максимально самодостаточным и его зависимости должны быть явными. Если этого нет — его просто неудобно использовать.

Синглтон сервис в ларавеле по умолчанию обращается к сессии.
Если сервис зависит от абстрактной сессии (в данный момент это просто HTTP-сессия), то модель, зависящая от этого сервиса зависит и от сессии.
Так что именно к сессии она и обращается (хоть и не напрямую).


И да, можно сделать так


Auth::login($user);
// и совсем в другой части проекта
$u->isCurrentUser();

И всё это будет работать. Но это будет понятно только тому, кто изначально писал этот код.

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

Проблема в том, что с одной и той же моделью можно работать разными способами — запросы будут разные.


Сейчас она обращается к сессии.


Потом понадобилось сделать консольную команду (не тест), чтобы сделать что-нибудь с пользователями. Как в данном случае будет работать Auth::user()? Как сделать пользователя авторизованным?

Вы правда не видите проблем в том, что модель напрямую обращается к параметрам запроса?

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


public function isCurrentUser()
{
    return ($this->id === current_user()->id);
}

Если я вижу такое у кого-то из коллег, то требую переписать.


У вас current_user() как определяется? Я полагаю, берётся из сессии, кук, заголовка — это всё накроется медным тазом, когда захочется вызвать данный код из консоли.


Почему данные HTTP-запроса (или хз вообще какого запроса — тут не ясно) протекают в модель, которая вообще не должна быть привязана ни к какому интерфейсу взаимодействия с приложением? И как это тестировать?




    event(new UserCreated($user));

    send_email_to_admins('У нас новый пользователь!', 'emails.user-registered', [
        'newUser' => $user,
    ]);

    // Отправим пользователю сообщение-приветствие
    send_email_to_user('welcome', $user);

    session(['first-login' => true]);

Тут я вообще не понял, такое ощущение, что прикрутили события после того как сделали отправку писем, но забыли сделать рефакторинг. Если у вас идёт вызов двух функций, от которых тем более не требуется ответ, почему нельзя их же обработать через этот event?


И останется только:


 event(new UserCreated($user));

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

Это, конечно, удобно.


Но у нас есть некий "use", потому лямбды надо делать либо с ним, либо вообще отказываться от этого "use". Т.к. иначе это будет ещё одна несогласованность в языке.

Лично мне не нравятся истории с самим Тиньковым.

Мошенник — это тот, кто нарушает определённую уголовную статью. Какие статьи в какой стране — это другой вопрос.
Если вы собираетесь использовать пар для отопления и получения горячей воды, то при условии нахождения вдалеке от населённых пунктов нужно также учитывать потери при передаче этой самой воды. Я вообще не уверен что имеет смысл тянуть теплотрассу длиной в 100-200 км.

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

Он вообще такой интересный.
Вот тут можно проследить интересную логику: https://twitter.com/search?f=tweets&vertical=default&q=LTS%20from%3Ataylorotwell&src=typd


LTS is sort of an anti-pattern IMO.

I am not a fan of LTS in general. Prefer people take a couple days per year to stay updated

А до этого:


Forge currently designed for 14.04 LTS… will update to 16.04 LTS when its available

А чего ж не 14.10, 15.04, 15.10? Хорошо же несколько дней в году тратить на обновления! Быть на острие! Тем более когда LTS — антипаттерн!

А ещё можно сделать так, чтобы в команде был сервис-локатор, из которого бы лениво подгружался единственный объект и выполнялся — вынести всю логику из команды. По факту, это будет то же самое, о чём сейчас и говорим в ларавеле. В данном случае сервис локатор будет выступать в роли фабрики — и не более.


Только одно дело — применять эти фасады (как и сервис-локаторы) в контроллерах и командах, а другое — погружать их куда-нибудь далеко-далеко в бизнес логику.

Вы правы, фасады это не статика. Это классический сервис-локатор.
И правильнее пример переписать так:


public function some()
{
    return $this->serviceLocator->get('some.any', 23);
}

В этом нет ничего сильно криминального за исключением того, что это крайне сильно повышает связанность кода.


Также лично я никогда не понимал радостей внедрения зависимостей прямо в методы. Это несколько нелогично перемешивать зависимости (которые могут меняться, добавляться и убираться) и аргументы — нет стабильного интерфейса.

Information

Rating
Does not participate
Location
Тула, Тульская обл., Россия
Date of birth
Registered
Activity