Specification pattern

Обсуждаем, как правильно строить приложения
Ответить
Melodic
Сообщения: 87
Зарегистрирован: 2016.05.11, 17:43
Откуда: Луганск

Specification pattern

Сообщение Melodic »

Всем привет!


Есть такой хендлер

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



    public function handle(RequestWithdrawTransactionCommand $command): WithdrawTransaction
    {

        $currency = Currency::get($command->getCurrency());

        $user = $this->userRepository->get($command->getUserId());

        $currencyConfig = $this->currencyConfigRepository->getByCurrency($currency);
        $amount = Amount::fromString($command->getAmount());


        if ($this->withdrawChecker->isReachLimit($command->getUserId(), $amount, $currency)) {
            throw new LimitException('Withdraw limit');
        }
        if ($currencyConfig === null) {
            throw new \DomainException('Currency config not found');
        }
        if ($amount->isNegative() || $amount->isZero()) {
            throw new \DomainException('Wrong amount value');
        }
        if (!$currencyConfig->isWithdrawEnabled()) {
            throw new \DomainException('Withdraw disabled');
        }

        $currencyConfig->getWithdrawLimit()->assertAmount($amount);

        $transaction = WithdrawTransaction::requestWithdraw(
            $user,
            $command->getAddress(),
            $amount,
            $currency,
            $currencyConfig->getWithdrawFee(),
            $this->confirmationCodeGenerator
        );

        $this->transactionRepository->add($transaction);

        return $transaction;

    }

Напрягает куча if'ов. Есть мнение, что здесь применим паттерн Specification для валидации бизнес правил. Но пока не могу сообразить как сделать, т.к. все проверяемые объекты разных типов, а метод isSatisfiedBy() y спецификации по идее должен принимать только один параметр одно типа. Как быть в этом случае? Возможно я ошибаюсь о приминении здесь Specification?
Melodic
Сообщения: 87
Зарегистрирован: 2016.05.11, 17:43
Откуда: Луганск

Re: Specification pattern

Сообщение Melodic »

Также, до того как команда попадет в хендлер она проходит через ряд валидаторов:

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

class OperationWithUsersValidatorMiddleware implements Middleware
{


    /**
     * @param object $command
     * @param callable $next
     * @return mixed
     * @throws SameUserException
     */
    public function execute($command, callable $next)
    {
        if ($command instanceof OperationWithUniqueUsersCommand && $command->firstUserId() === $command->secondUserId()) {
            throw new SameUserException('Same user');
        }

        return $next($command);
    }
}

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

class OperationValidatorMiddleware implements Middleware
{

    /** @var UserRepository */
    private $userRepository;

    /**
     * OperationValidatorMiddleware constructor.
     * @param UserRepository $userRepository
     */
    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }


    /**
     * @param object $command
     * @param callable $next
     * @return mixed
     * @throws OperationBlockedException
     * @throws UserException
     */
    public function execute($command, callable $next)
    {
        if ($command instanceof OperationCommand) {
            $user = $this->userRepository->get($command->operationUserId());
            $operation = sprintf('is%sBlocked', ucfirst($command->operation()));
            if (!method_exists($user, $operation) || $user->{$operation}()) {
                throw new OperationBlockedException($command->operation());
            }
        }
        return $next($command);
    }
}
и еще парочка похожих. И вот это размазывание тоже не нравится. Возможно, стоит спецификацию для каждой отдельной команды делать?
anton_z
Сообщения: 483
Зарегистрирован: 2017.01.15, 15:01

Re: Specification pattern

Сообщение anton_z »

Здравствуйте! На мой взгляд вы слишком увлеклись паттернами. Сегодня выходной, потрачу, пожалуй, немного времени и напишу свое видение решения проблемы кучи if-ов (хотя их не так уж и много)). Specification тут вам особо не поможет, так как проблема состоит в том, что у вас клиентский код как бы опрашивает объекты снаружи и принимает решения, а надо чтобы объекты сами принимали решения. Даже если вы примените Specification, классы specification все равно будут содержать много if-ов, так как вам понадобится куча интроспекций по типу. На мой взгляд If-вы рефакторятся полиморфизмом.
Решение примерное, в вашем решении может быть больше или меньше декораторов, в зависимости от того, как разделите проверки семантически.

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


public function handle(RequestWithdrawTransactionCommand $command): WithdrawTransaction
{

    $user = $this->userRepository->get($command->getUserId());
    
    if ($user === null) {
        throw new DomainException('User is not found.')ж
    }

    $currency = Currency::get($command->getCurrency());

    $amount = Amount::fromString($command->getAmount());
    
    $invoice = new LimitAwareInvoice(
        new CurrencyAvailabilityAwareInvoce(
            new DefaultInvoice(
                $amount,
                $currency,
                $command->getAddress(),
                $user,
                $this->transactionRepostory,
                $this->currencyConfigRepository,
                $this->confirmationCodeGenerator
            ),
            $this->currencyConfigRepository
        ),
        $this->withdrawChecker
    );

    return $invoice->makeTransaction();
}

interface Invoice
{

    public function amount(): Amount;

    public function currency(): Currency;

    public function user(): User;

    public function makeTransaction(): WithdrawTransaction;

}

class DefaultInvoice implements Invoice
{

    /**
     * @var Amount
     */
    private $amount;

    /**
     * @var Currency
     */
    private $currency;

    /**
     * @var string
     */
    private $address;

    /**
     * @var User
     */
    private $user;

    /**
     * @var TransactionRepostory
     */
    private $transactionRepository;

    /**
     * @var CurrencyConfigRepository
     */
    private $currencyConfigRepository;

    /**
     * @var ConfirmationCodeGenerator
     */
    private $confirmationCodeGenerator;

    /**
     * DefaultInvoice constructor.
     *
     * @param Amount $amount
     * @param Currency $currency
     * @param string $address
     * @param User $user
     * @param TransactionRepostory $transactionRepository
     * @param CurrencyConfigRepository $currencyConfigRepository
     * @param ComnfirmationCodeGenerator $confirmationCodeGenerator
     */
    public function __construct(
        Amount $amount,
        Currency $currency,
        string $address,
        User $user,
        TransactionRepostory $transactionRepository,
        CurrencyConfigRepository $currencyConfigRepository,
        ComnfirmationCodeGenerator $confirmationCodeGenerator
    ) {
        $this->amount                    = $amount;
        $this->currency                  = $currency;
        $this->address                   = $address;
        $this->user                      = $user;
        $this->transactionRepository     = $transactionRepository;
        $this->currencyConfigRepository  = $currencyConfigRepository;
        $this->confirmationCodeGenerator = $confirmationCodeGenerator;
    }


    public function makeTransaction(): WithdrawTransaction
    {

        $currencyConfig = $this->currencyConfigRepostitory->get($this->currency);

        if ($currencyConfig === null) {
            throw new DomainException('Currency config is not found.');
        }

        if ($this->amount->isNegative() || $this->amount->isZero()) {
            throw new \DomainException('Wrong amount value');
        }

        return $this->transactionRepository->add(
            $this->user,
            $this->address,
            $this->amount,
            $this->currency,
            $currencyConfig,
            $this->confirmationCodeGenerator
        );
    }


    public function amount(): Amount
    {
        return $this->amount;
    }


    public function currency(): Currency
    {
        return $this->currency;
    }


    public function user(): User
    {
        return $this->user;
    }

}


abstract class DecoratedInvoice implements Invoice
{

    /**
     * @var Invoice
     */
    protected $invoice;

    /**
     * DecoratedInvoice constructor.
     *
     * @param Invoice $invoice
     */
    public function __construct(Invoice $invoice)
    {
        $this->invoice = $invoice;
    }


    public function amount(): Amount
    {
        return $this->invoice->amount();
    }


    public function currency(): Currency
    {
        return $this->invoice->currency();
    }

    public function user():  User
    {
        return $this->invoice->user();
    }

}

class CurrencyAvailabilityAwareInvoice extends DecoratedInvoice
{

    /**
     * @var CurrencyConfigRepository
     */
    private $currencyConfigRepository;

    /**
     * CurrencyAvailabilityAwareInvoice constructor.
     *
     * @param Invoice $invoice
     * @param CurrencyConfigRepository $currencyConfigRepository
     */
    public function __construct(
        Invoice $invoice,
        CurrencyConfigRepository $currencyConfigRepository
    ) {
        $this->currencyConfigRepository = $currencyConfigRepository;
        parent::__construct($invoice);
    }

    public function makeTransaction(): WithdrawTransaction
    {

        $currencyConfig = $this->currencyConfigRepository->getByCurrency($this->invoice->currency());

        if ($currencyConfig === null) {
            throw new DomainException('Currency config is not found.');
        }

        if (!$currencyConfig->isWithdrawEnabled()) {
            throw new \DomainException('Withdraw disabled');
        }

        $currencyConfig->getWithdrawLimit()->assertAmount($this->invoice->amount());


        return $this->invoice->makeTransaction();

    }


}


class LimitAwareInvoice extends DecoratedInvoice
{

    /**
     * @var WithdrawChecker
     */
    private $withdrawChecker;


    /**
     * LimitAwareInvoice constructor.
     *
     * @param Invoice $invoice
     * @param WithdrawChecker $withdrawChecker
     */
    public function __construct(
        Invoice $invoice,
        WithdrawChecker $withdrawChecker
    ) {
        $this->withdrawChecker = $withdrawChecker;
        parent::__construct($invoice);
    }

    public function makeTransaction(): WithdrawTransaction
    {

        if ($this->withdrawChecker->isReachLimit($this->invoice->user()->id(), $this->invoice->amount(), $this->invoice->currency())) {
            throw new DomainException('Withdraw limit');
        }

        return $this->invoice->makeTransaction();

    }

}

Подумайте, подойдет ли вам это, или лучше оставить один не такой уж и сложненький хендлер?

C if в Middleware ничего сделать особо не получится, Вы сами выбрали командную шину, а она фактически подразумевает интроспекции по типу.
Ответить