Is a "guard" method throwing an exception good practice?

Question

Would the following code be considered a "good" practice?

It's the controller of an RPC endpoint of a package. The idea is to easily override/extend the validation or authorization for a specific project that includes the package.

Could you say that these are guard clauses or methods? Is it a good idea to have a method which only purpose is to check something and throw an exception if something goes wrong?

The code looks clean to me, but I would to get some advice on this :)

public function doSomethingWithCustomer() {
    try {
        $this->validate();
        $this->authorize();
        $customer = $this->resolveCustomer();
        $customer->doSomething();
    } catch (HttpException $e) {
        $this->logger->error($e->getMessage());
        return $this->errorResponse($e->getStatusCode(), $e->getMessage());
    }
}

protected function validate()
{
    // Validate input

    if (!$valid) {
        throw new BadRequestHttpException('Invalid email address');
    }
}

protected function authorize()
{
    // Do some authorization checking

    if ($notAuthorized) {
        throw new AccessDeniedHttpException('Not authorized');
    }
}

protected function resolveCustomer()
{
    $customer = Customer::load(1);

    if (is_null($customer) {
        throw new NotFoundHttpException('Customer not found');
    }

    return $customer;
}

This sort of thing is seen throughout the JDK, for example as between `Socket` and `SocketImpl`, or in many methods which ultimately delegate to JNI after doing all the obvious checks. — user207421, Feb 14 '18 at 23:49
Basically this is the purpose of the Exceptions. To short circuit the normal flow in case of something going wrong. Although i would move the authorization checking to different class and override it if necessary. — Nick Polyderopoulos, Feb 15 '18 at 00:26

score 2 · Answer 1 · answered Apr 11 '18 at 03:18

2

No, this is a bad practice for the following reasons:

You should never catch a guard clauses exception (it's part of the fail-fast methodology)
A method can't contains only guard clauses
Guard clauses should not validate business logic

Here is an article of mine on guard clauses. I hope it will help you to better understand them.

answered Apr 11 '18 at 03:18

Maxime Gélinas

2,202
2
18
35

I'm not sure that's so true... See Apple's documentation on JSON for example, here: https://developer.apple.com/swift/blog/?id=37 – Ishay Peled Aug 11 '20 at 19:09
I'm saying that throwing an exception when you know you will catch it later is wrong. Throwing from guard clauses is perfectly fine, but you shouldn't catch guard clause exceptions. The either pattern or option pattern are better options. – Maxime Gélinas Aug 12 '20 at 19:56
Gotcha, I did not understand this the first time – Ishay Peled Aug 23 '20 at 08:50

Is a "guard" method throwing an exception good practice?

1 Answers1