Оценка кода

[Yoskaldyr]

@grigori, А как можно использовать фабрику, если в месте выброса исключения (модель) явно не известно какой контроллер/экшн и в какой части этого экшена (a один и тот же метод модели может вызваться больше одного раза в одном экшене).

 

[Adelf]

если в месте выброса исключения (модель) явно не известно какой контроллер/экшн и в какой части этого экшена (a один и тот же метод модели может вызваться больше одного раза в одном экшене).

Боюсь, неправильная у тебя модель какая-то. Юзеру наплевать на наши контроллеры и экшены. Ему важно получить сообщение, что не получилось сделать и по какой причине. Нормальные Exception с этим справятся.

И да, тут люди подумали что у меня в Exception handler какой-нибудь огромный switch или куча if. Все гораздо проще.

    /**
     * Render an exception into an HTTP response.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Exception  $e
     * @return \Illuminate\Http\Response
     */
    public function render($request, Exception $e)
    {
        if($e instanceof INotFoundException)
        {
            // makes 404 error if something haven't found
            $e = new NotFoundHttpException($e->getMessage(), $e);
        }

        if($e instanceof IErrorMessageException)
        {
            return redirect()->back()->with('error', $e->getMessage())->withInput();
        }

        return parent::render($request, $e);
    }

Всего два общих варианта ошибки. Exception проверяется на маркеры - Чтото не нашлось и какая-то ошибка логики, которую можно показать пользователю.
Ну и проект просили делать быстро, поэтому напрямую getMessage() показывается юзеру. И оба интерфейса пусты и работают просто как маркеры. Если есть время, конечно, надо более ответственно подойти.
Пример Exception:

class EmptyIndustriesException extends \Exception implements IErrorMessageException 
// Обычная ошибка бизнес логики. 
{
    public function __construct()
    {
        parent::__construct('At least one industry should be chosen');
    }
}

class EntityNotFoundException extends \Exception implements INotFoundException
// Опять-таки из-за сжатых сроков, один EntityNotFoundException на все случаи. Хотя... мож оно и правильно.
{
    /**
     * @var string
     */
    private $entityType;
    /**
     * @var string
     */
    private $condition;
    /**
     * @var string
     */
    private $conditionValue;

    /**
     * EntityNotFoundException constructor.
     * @param string $entityType
     * @param string $condition
     * @param string $conditionValue
     */
    public function __construct($entityType, $condition, $conditionValue)
    {
        parent::__construct("Entity not found");

        $this->entityType = $entityType;
        $this->condition = $condition;
        $this->conditionValue = $conditionValue;
    }
    // Тут еще геттеры к тем трем свойствам, но они только место занимают
}

Я проектом особо не горжусь. Много чего не очень правильно сделано. Был то на нем 2 месяца. Но в общем код весьма чистый и SRP-подобный.

 

[grigori]

@grigori, А как можно использовать фабрику, если в месте выброса исключения (модель) явно не известно какой контроллер/экшн и в какой части этого экшена.

Это потому что у тебя в контроллере логика приложения. Теоретически в контроллере должна быть авторизация действия, вызов $TopicMoverService->move($from,$to); и вывод результата операции - редирект или json, и все.

 

[scorpion-ds]

Просто если нужен полный контроль за текстом возвращаемой пользователю ошибки, то придется или использовать много лапши из try catch в контроллерах

Я как-то уже давненько не использовал try catch в контролерах, только в сервисах, в самих контролерах я только проверяю разультат работы методов сервисов и в зависимости от результат формирую Response.

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