Запихнуть HttpException в сервис?

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

Запихнуть HttpException в сервис?

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

Очищаю контроллеры от лишнего кода, выношу всё по максимуму в сервисы.
Встал вопрос, можно ли выносить HttpException в сервис?
Допустимо ли это по хорошей архитектуре, или это должно привести к проблемам?

Вот два варианта.

1. В контроллере только вызов сервиса. Исключение будет брошено в сервисе.

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

class ArticleController extends Controller
{
    public function actionView($id)
    {
        $article = ArticleFinder::findPublishedOrDie($id);
        return $this->render('view', ['model' => $article]);
    }
}

class ArticleFinder
{
    public static function findPublishedOrDie($id)
    {
        $article = Article::find()->where(['published' => true]);
        if (empty($article)) {
            throw new HttpNotFoundException;
        }
        return $article;
    }
}
2. В контроллере вызов сервиса, проверка и исключение. В сервисе только выборка.

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

class ArticleController extends Controller
{
    public function actionView($id)
    {
        $article = ArticleFinder::findPublished($id);
        if (empty($article)) {
            throw new HttpNotFoundException;
        }
        return $this->render('view', ['model' => $article]);
    }
}

class ArticleFinder
{
    public static function findPublished($id)
    {
        return Article::find()->where(['published' => true]);
    }
}
Вариант 1. Плюс - код контроллера проще. Минус - сервис завязан на HttpException, возможно это вообще архитектурно неправильно.
Вариант 2. Плюс - красивый сервис, не подкопаться. Минус - контроллер стал толще.

Что лучше? Кто как решает эту задачу? Прошу мнений.
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение ElisDN »

Второй вариант.

А так сервис может кинуть свой NotFoundException. Не Http.
zelenin
Сообщения: 10596
Зарегистрирован: 2013.04.20, 11:30

Re: Запихнуть HttpException в сервис?

Сообщение zelenin »

http - это уровень представления приложением данных, а сервис - это о работе с данными, и он не знает в каком виде эти данные будут представлены - http, stdout, tcp. Поэтому два разных слоя не смешиваем.
Аватара пользователя
SiZE
Сообщения: 2813
Зарегистрирован: 2011.09.21, 12:39
Откуда: Perm
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение SiZE »

ElisDN писал(а): 2017.04.23, 21:07 А так сервис может кинуть свой NotFoundException. Не Http.
Задам смежный вопросик. Вот например сервис бросает какой-то LogicException, но с разными сообщениями. Как в Yii2 бросить эксепшен с человеческим текстом? Приведу, пример, как я это вижу, но может поправите.

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

class ArticleFinder
{
    /**
      * Article not found
      */
    const CODE_NOT_FOUND = 1;

    public static function findPublishedOrDie($id)
    {
        throw new DomainException("Article not found", self::CODE_NOT_FOUND);
    }
}

class ArticleController extends Controller
{
    public function actionView($id)
    {
        try {
            $article = ArticleFinder::findPublishedOrDie($id);
        } catch (\DomainException $e) {
            if ($e->getCode() === ArticleFinder::CODE_NOT_FOUND) {
                // нормально? :)
                throw new HttpException(404, "Статья не найдена.");
            }
        }
        return $this->render('view', ['model' => $article]);
    }
}
Или это не приемлемо и на каждый случай свое исключение добавлять? По аналогии с наследниками yii\web\HttpException.
Или создать эксепшен и прописать там константы кодов? (последнее мне кажется будет не удобным и дичью попахивает :))
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение ElisDN »

SiZE писал(а): 2017.04.24, 09:41 Как в Yii2 бросить эксепшен с человеческим текстом?
Если тексты в оригинальных эксепшенах на английском, то можно легко преобразовывать через переводы:

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

try {
    $this->service->...;
} catch (\DomainException $e) {
    throw new BadRequestHttpException(Yii::t('exceptions', $e->getMessage()), 0, $e);
}
А для поиска можно либо делать orDie :

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

public function findModel($id)
{
    try {
        return ArticleFinder::findPublished($id);
    } catch (NotFoundException $e) {
        throw new NotFoundHttpException(Yii::t('exceptions', $e->getMessage()));
    }
}
либо не делать:

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

public function findModel($id)
{
    if (!$model = ArticleFinder::findPublished($id)) {
        throw new NotFoundHttpException('Статья не найдена');
    }
    return $model;
}
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

Спасибо за ответы.

Придумал ещё такой вариант, с добавлением дополнительного класса на обслуживание контроллеров.
Плюсы - и контроллер освободил, и файндер не испортил. Небольшой минус - классов больше стало.

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

class ArticleController extends Controller
{
    public function actionView($id)
    {
        $article = ArticleHttpService::findPublishedOrDie($id);
        return $this->render('view', ['model' => $article]);
    }
}

class ArticleFinder
{
    public static function findPublished($id)
    {
        return Article::find()->where(['published' => true]);
    }
}

class ArticleHttpService
{
    public static function findPublishedOrDie($id)
    {
        $article = ArticleFinder::findPublished($id);
        if (empty($article)) {
            throw new HttpNotFoundException;
        }
        return $article;
    }
}
Аватара пользователя
SiZE
Сообщения: 2813
Зарегистрирован: 2011.09.21, 12:39
Откуда: Perm
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение SiZE »

Nex-Otaku писал(а): 2017.04.25, 09:43Придумал ещё такой вариант, с добавлением дополнительного класса на обслуживание контроллеров.
ПМСМ оверхед
zelenin
Сообщения: 10596
Зарегистрирован: 2013.04.20, 11:30

Re: Запихнуть HttpException в сервис?

Сообщение zelenin »

SiZE писал(а): 2017.04.25, 14:04
Nex-Otaku писал(а): 2017.04.25, 09:43Придумал ещё такой вариант, с добавлением дополнительного класса на обслуживание контроллеров.
ПМСМ оверхед
бред вообще, а не подход
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

Аргументы?
Аватара пользователя
SiZE
Сообщения: 2813
Зарегистрирован: 2011.09.21, 12:39
Откуда: Perm
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение SiZE »

Nex-Otaku писал(а): 2017.04.25, 19:51 Аргументы?
http://www.yiiframework.ru/forum/viewto ... 14#p215614

Самый последний вариант тоже что и первый.
zelenin
Сообщения: 10596
Зарегистрирован: 2013.04.20, 11:30

Re: Запихнуть HttpException в сервис?

Сообщение zelenin »

Nex-Otaku писал(а): 2017.04.25, 19:51 Аргументы?
какие еще аргументы? создал сервис вокруг другого сервиса, чтобы обернуть исключение? и хочешь теперь сказать, что второй сервис - это сервис http-слоя?
anton_z
Сообщения: 483
Зарегистрирован: 2017.01.15, 15:01

Re: Запихнуть HttpException в сервис?

Сообщение anton_z »

zelenin писал(а): 2017.04.25, 22:59
Nex-Otaku писал(а): 2017.04.25, 19:51 Аргументы?
какие еще аргументы? создал сервис вокруг другого сервиса, чтобы обернуть исключение? и хочешь теперь сказать, что второй сервис - это сервис http-слоя?
Мне это отдаленно напомнило перехват (декоратор) https://smarly.net/dependency-injection ... terception Только статический).
Избавьтесь от статики. Она не для этого нужна. То, в каком виде вы ее используете - зло нетестируемое.
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

zelenin писал(а): 2017.04.25, 22:59 создал сервис вокруг другого сервиса, чтобы обернуть исключение? и хочешь теперь сказать, что второй сервис - это сервис http-слоя?
Да. А что не так? Конкретнее? Оценка "всё неправильно" это хорошо, но хотелось бы ещё услышать, почему неправильно.
Аватара пользователя
SiZE
Сообщения: 2813
Зарегистрирован: 2011.09.21, 12:39
Откуда: Perm
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение SiZE »

Nex-Otaku писал(а): 2017.04.27, 22:44 Да. А что не так? Конкретнее? Оценка "всё неправильно" это хорошо, но хотелось бы ещё услышать, почему неправильно.
Я же дал ссылку даже на первое сообщение Зеленина, где он все написал. Что ты еще хочешь услышать? Ты принципиально это игнорируешь?
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

SiZE, там написано "это разные слои, поэтому их не смешиваем". А мне хочется знать, почему. Почему не сделать так? К каким проблемам это может привести? Пока что слышу только "так нельзя, потому что неправильно". Но для меня это звучит неубедительно. Убедительно будет пример, в котором вылезает какой-то косяк при таком подходе.

Я же не утверждаю, что вы не правы. Я не спорю. Просто хочу разобраться и сам понять, а не слепо положиться на чей-то авторитет. Рано или поздно я и сам разберусь, но с вашей помощью это будет быстрее. Поэтому и спрашиваю.

Суть задачи - я просто хочу максимально облегчить контроллер. Мне не нравится, что на одно действие нужно писать четыре строчки кода, я хотел бы уместить его в одну строку. Чтобы и читаемо, и красиво, и архитектуру при этом не испоганить.

Вариант с выбросом не-HTTP исключения в сервисе, конечно хороший, но получается, что нужно всё равно где-то такие исключения отдельно ловить и обрабатывать, в ErrorController или ещё где-то. Мне это не нравится, потому что нужно всё время помнить, что оно где-то там в третьем месте обработается, вести там учёт всех этих специфичных исключений и т.д.

Вариант с выбросом исключения в самом контроллере мне не нравится из-за многословности. Хотелось бы сократить код.

Также придумался ещё один вариант:

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

class ArticleController extends Controller
{
    public function actionView($id)
    {
        $article = ArticleFinder::findPublished($id);
        Assert::notEmpty($article, new HttpNotFoundException);
        return $this->render('view', ['model' => $article]);
    }
}

class ArticleFinder
{
    public static function findPublished($id)
    {
        return Article::find()->where(['published' => true]);
    }
}

class Assert
{
    public static function notEmpty($value, $exception)
    {
        if (empty($value)) {
            throw $exception;
        }
    }
}
Две строки, но хотя бы уже не четыре.
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение ElisDN »

Nex-Otaku писал(а): 2017.04.28, 09:25 Мне не нравится, что на одно действие нужно писать четыре строчки кода...
...и Вы вместо этого для каждого действия делаете целый класс?

А действие можно с четырёх до трёх строк сократить:

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

public function actionView($id)
{
    if (!$article = ArticleFinder::findPublished($id)) {
        throw new HttpNotFoundException;
    }
    return $this->render('view', ['model' => $article]);
}
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

ElisDN писал(а): 2017.04.28, 10:19
Nex-Otaku писал(а): 2017.04.28, 09:25 Мне не нравится, что на одно действие нужно писать четыре строчки кода...
...и Вы вместо этого для каждого действия делаете целый класс?
Не для каждого, но именно это действие буквально на каждом шагу в контроллерах. Получил - проверил - передал.
Аватара пользователя
delvin
Сообщения: 85
Зарегистрирован: 2009.11.13, 15:29

Re: Запихнуть HttpException в сервис?

Сообщение delvin »

А если для передачи между слоями использовать что-то типа Aura.Payload ?
Nex-Otaku
Сообщения: 831
Зарегистрирован: 2016.07.09, 21:07

Re: Запихнуть HttpException в сервис?

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

Не говорите мне таких страшных слов )

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

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

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

class ArticleController extends Controller
{
    use DieOnEmpty;
    
    public function actionView($id)
    {
        $article = ArticleFinder::findPublished($id);
        $this->dieOnEmpty($article);
        return $this->render('view', ['model' => $article]);
    }
}

class ArticleFinder
{
    public static function findPublished($id)
    {
        return Article::find()->where(['published' => true]);
    }
}

trait DieOnEmpty
{
    private function dieOnEmpty($value)
    {
        if (empty($value)) {
            throw new HttpNotFoundException;
        }
    }
}
Аватара пользователя
slavcodev
Сообщения: 3134
Зарегистрирован: 2009.04.02, 21:42
Откуда: Valencia
Контактная информация:

Re: Запихнуть HttpException в сервис?

Сообщение slavcodev »

Открой для себя Middleware (Action Filters в Yii).
Все что повторяется в контролерах, связанное с HTTP выноси туда:

- Empty и другая валидация $_GET параметров
- Валидация $_POST данных
- Отлов исключение уровня Applciation и конвертация в HTTP errors.
Жду Yii 3!
Ответить