Как кастомизировать ексепшены в общем методе?

Question

[Di Lee]

Есть такой метод в общем сервисе, которые используется в нескольких местах в коде проекта

public function checkParams(array $params) {
        foreach($params as $param) {
            if (!in_array($param, self::ARRAY_CONSTANT)) {
                throw new CustomException('Custom message');
            }
        }
    }

Как правильно кастомизировать ексепшены с точки зрения чистого кода, ооп подхода, чтобы при вызове с разных мест использовать свой кастомный ексепшен?

Возможны ли такие подходы или может быть есть другие варианты?:

1. CustomException и 'Custom message' передавать в виде параметров или опций, примерно так:

public function checkParams($params, string $customExceptionClassName, string $customMessage) {
      ...
     }

2. Либо правильнее использовать какой то маппинг ексепшенов(в привязке на какие-то кастомные типы переданные в опциях метода) или сделать через фабрику или что то еще?

Comments

[tukreb]

Вы снова пытаетесь сделать супер класс который умеет делать всё?
Выкидывайте эксепшен где он произошёл. тогда и велосипедить не придётся, и читаемость кода будет большая. А если вы какой то валидатор делаете, то просто используйте готовые решение от симфони.

[Василий Банников]

чтобы при вызове с разных мест использовать свой кастомный ексепшен?

Это уже пахнет каким-то говнокодом. Зачем это нужно?

[tukreb]

Василий Банников, он понимает буквально принцип DRY

[Di Lee]

Василий Банников, спасибо. В целом ответ понятен.
tukreb ну хорошо что ты все понимаешь не буквально

[sl0]

Я вообще не понимаю зачем в данном случае разные эксепшены.
Сделать один ParamException и кидать его в этом методе, а в message прикладывать ключ неверного параметра. Больше тут ничего не нужно.

[Di Lee]

sl0, да я понял уже принцип. Можно общий ексепшен, можно и какой-то boolean возвращать. Главное что разруливать кастомизацию уже выше

[Василий]

Как я вижу эту проблему

<?php

class A
{
    function run()
    {
        throw new Exception("Common error", 42);
    }
}


class B
{
    /**
     * @var $a A
     */
    public $a;

    function run()
    {
        try {
            $this->a->run();
        } catch (Exception $exception) {
            if ($exception->getCode() == 42) {
                throw new RuntimeException("Custom error");
            }
        }
    }
}

Answer 1

[Ипатьев]

Первый вариант - это однозначно глупость. Какое-то совсем уже натягивание процедурного подхода на ООП.
Второй какой-то невнятный, про него ничего нельзя сказать толком.

В общем случае, как правильно написал sl0 в комментарии, кидать надо ОДИН тип исключения, но с разными сообщениями.

В данном конкретном случае, поскольку речь идёт о валидации, то никакое исключение в принципе кидаться не должно. Исключение кидается в случае, если функция не может выполнить свою работу. Но для функции checkParam() невалидный параметр НЕ ЯВЛЯЕТСЯ исключительной ситуацией. Потому что роль этой функции - как раз и определить валидность переданного параметра. В случае неверных данных функция должна записать сообщение об ошибке во внутренний массив ошибок класса-валидатора и вернуть false.

spoiler

Исключение в случае неверных параметров может кидать например функция сохранения в БД. Потому что её задача - сохранить переданные данные, а не валидировать их. И в случае, если функция не может сохранить данные - она должна кинуть исключение, в том числе из-за невалидности данных.

Comments

[Di Lee]

Тут можно и к boolean придраться. На сколько я знаю, если возвращать booleаn, то метод должен начинаться с "is" и называться примерно как-то "isValidParams".

[sl0]

Di Lee, Путаешь. Если метод начинается с is, то да, он должен возвращать true|false, но если метод возвращает true|false это вовсе не значит, что он должен начинаться с is.
Можно, конечно, извратиться и назвать как-то типа IsParamsCorrected, но это вряд ли кому нужно.

[sl0]

В данном конкретном случае, поскольку речь идёт о валидации, то никакое исключение в принципе кидаться не должно.

Кстати да. Что-то я не подумал, что в таком случае если неверных параметров несколько, то мы узнаем только о первом. По-хорошему я бы возвращал true если все в порядке или массив неверных параметров. Исключения в данной ситуации, действительно, ни к чему.

[Di Lee]

sl0, то есть checkParams должен возвращать mixed данные (либо массив, либо boolen). Возможно тогда правильнее будет, чтобы метод возвращал какой то объект CheckeResult (допустим даже это будет self), где будет либо данные невалидные либо $checkeResult->isValid() == true? по крайней мере это будет более строгая типизация и чуть ближе к ООП подходу?

[Ипатьев]

Зачем эта функция возвращает массив? Она как-то меняет переданные в неё данные?
Функция должна делать что-то одно - либо проверять данные, либо изменять их

[sl0]

Di Lee, Сложно говорить, не зная конкретики.
Например, если эта функция нужна для проверки параметров апи, чтобы потом сообщить, что введено неверно, то имеет смысл всегда возвращать массив errors, а затем действовать в зависимости от \count($errors). Тогда можно обойтись и без mixed.
Если же она должна быть прям универсальной, то тут возможен и вариант с объектом и много других.
Пока, по представленному коду, это все похоже на обычный валидатор. Соответственно все рассуждения относятся именно к валидатору.

[Di Lee]

sl0, ясно. На счет mixed данных в ответе метода в аспекте ООП - тут все же вопрос остается. Как то это не по феншую, что ли. Если есть на эту тему информация - скиньте почитать.

[Ипатьев]

Я писал выше.

class someValidator
{
    protected $errors = [];
    public function checkParams($params): bool
    {
        if(...)
        {
            $this->errors[] = "some error";
            return false;
        }
        return true;
    }
    public function getErrors():array
    {
        return $this->errors;
    }
}

Валидация отдельно, ошибки - отдельно

[Di Lee]

Ипатьев, спасибо