Архитектурное решение MVC

Обсуждаем, как правильно строить приложения
Ответить
medinsky
Сообщения: 4
Зарегистрирован: 2018.03.02, 11:25

Архитектурное решение MVC

Сообщение medinsky »

Здравствуйте, подскажите пожалуйста, что я делаю не так...
Хочу сделать так, как подсказали в этом видео-уроке https://youtu.be/WL0-bd2Afho?t=385 с 6.25 по времени.
Сказали чтобы получалось лучше, чтобы проще было потом поддерживать и расширять приложение, стоит выделять какую то бизнес логику в отдельные классы и не помещать её ни в модели, ни в контроллеры, ни тем более во вьюхи.

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

Раньше в модели был экшн:

Код: Выделить всё

public function actionDisable($id)
    {
        $model = $this->findModel($id);
        $model->disable = 1;
        $model->save();
        return $this->redirect(Yii::$app->request->referrer);
    }
и аналогичный actionEnable($id). Всё работало в пределах этой модели.

Я решил, т.к. у меня разные модели содержат одно и тоже действие actionDisable/Enable вынести это в отдельный класс, к примеру ModelDeleter, реализовав его так:

Код: Выделить всё

namespace app\models;

use yii\db\ActiveRecord;

class ModelDeleter
{
    const STATUS_ENABLED = 0;
    const STATUS_DISABLED = 1;
    protected $model;

    public function __construct(ActiveRecord $model) {
        $this->model = $model;
    }

    public function disable() {
        $this->model->disable = self::STATUS_DISABLED;
        $this->model->save();
    }

    public function enable() {
        $this->model->disable = self::STATUS_ENABLED;
        $this->model->save();
    }
}
в примере там вместо ActiveRecord стоял конкретный класс Client, я пробовал ставить и конкретный класс, а не ActiveRecord и всеравно не работало ничего.

внутри контроллера я писал аналогично, как в видео:

public function actionDisable($id)
{
$serviceCenter = $this->findModel($id);
Yii::createObject(ModelDeleter::class, [$serviceCenter])->disable();
return $this->redirect(Yii::$app->request->referrer);
}

и оно не работало. В тоже время в model в ModelDeleter попадает нужная модель, но она не устанавливается как disable, как ни крути.
Подскажите что можно тут исправить, чтобы можно было передавать в такой класс модель любого типа, которая содержит поле disable и чтобы можно было этой ModelDeleter выполнять disable и enable над переданной моделью. Спасибо
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Архитектурное решение MVC

Сообщение Nex-Otaku »

Приведённый код работает. Значит, дело не в нём, а в том коде, который вы не привели.

Скорее всего атрибут не прописан в модели. Проверяйте что в "rules" модели написано. Есть там "disable"? Или, может, вы сценариями пользуетесь?
anton_z
Сообщения: 483
Зарегистрирован: 2017.01.15, 15:01

Re: Архитектурное решение MVC

Сообщение anton_z »

Я бы не стал писать таких deleterов, это все возврат к процедурном у стилю.

Код: Выделить всё


class Client extends ActiveRecord
{

public function softDelete()
{
    $this->status = self::DEL;
    $this->save();

}
}


Экземпляр модели также как и раньше в контроллере можно вытащить.
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Архитектурное решение MVC

Сообщение Nex-Otaku »

Автор написал, что именно от размещения такого кода в модели он и хотел уйти.
medinsky
Сообщения: 4
Зарегистрирован: 2018.03.02, 11:25

Re: Архитектурное решение MVC

Сообщение medinsky »

Nex-Otaku писал(а): 2018.03.02, 13:44 Приведённый код работает. Значит, дело не в нём, а в том коде, который вы не привели.

Скорее всего атрибут не прописан в модели. Проверяйте что в "rules" модели написано. Есть там "disable"? Или, может, вы сценариями пользуетесь?
Здравствуйте, спасибо вам за совет. Дело действительно было в rules.
вначале поставил

Код: Выделить всё

 $model->save(false); 
и сохранял без проверок правил валидации, и в итоге оказалось, что CRUD генератор моё поле из базы данных типа

Код: Выделить всё

tinyint[1]
сгенерировал как

Код: Выделить всё

['string', 'max' =>1]
, я поставил boolean вместо этого и код сработал! Помогло :)

И пожалуйста подскажите, это правильный подход так выносить бизнес-логику из моделей или это плохая практика? Как лучше делать?
chesar
Сообщения: 514
Зарегистрирован: 2013.04.10, 17:49

Re: Архитектурное решение MVC

Сообщение chesar »

Бизнес логику, да. Тут понимается что, например, при удалении клиента, надо отправить ему письмо, его поместить в архив за текущий год и дать разовый промокод на 30%. Вот это в модели размещать не надо. А приведённый вами делитер, оперирует сущностью. Это должно остаться там.
anton_z
Сообщения: 483
Зарегистрирован: 2017.01.15, 15:01

Re: Архитектурное решение MVC

Сообщение anton_z »

medinsky писал(а): 2018.03.02, 22:33 И пожалуйста подскажите, это правильный подход так выносить бизнес-логику из моделей или это плохая практика? Как лучше делать?
Я думаю, совсем неправильный. https://habrahabr.ru/post/224879/
Nex-Otaku писал(а): 2018.03.02, 16:28 Автор написал, что именно от размещения такого кода в модели он и хотел уйти.
Ну и зря. В сообществе C# эту проблему с поцедурным характером "сервисов" обнаружили еще лет пять назад, а php пусть на своих ошибках учится. https://habrahabr.ru/post/158277/#comment_5418677
В приведенном примере налицо анемичная сущность.
chesar писал(а): 2018.03.02, 23:56 Бизнес логику, да. Тут понимается что, например, при удалении клиента, надо отправить ему письмо, его поместить в архив за текущий год и дать разовый промокод на 30%. Вот это в модели размещать не надо. А приведённый вами делитер, оперирует сущностью. Это должно остаться там.
Надо в sortDelete сгенерировать event, а в подписчиках отправить письмо и выдать промокод.

Код: Выделить всё


class Client extends CActiveRecord
{
    public function softDelete()
    {
         $this->deleted = true;
         $this->save();
         $this->addToArchive();
         $this->event_bus->trigger(new ClientDeletedEvent($this->email, $this->name);
         
    }

}

Ответить