3
\$\begingroup\$

I have backend project in which there's own "parent-to-all exception" exists, something like this (also I have InvalidArgumentException derived from this exception):

class CaliException extends \Exception {
    private $caliCode;

    public function __construct(string $message = "", string $caliCode = self::UNDEFINED_CODE,
                                int $code = 0, Throwable $previous = null) {
        parent::__construct($message, $code, $previous);

        $this->caliCode = $caliCode;
    }

    function getCaliCode(): string {
        return $this->caliCode;
    }
}

It's clear from code above, that purpose of custom class is ability to keep string codes. Why do I need string codes? Because I have HTTP API which works in the following way:

  1. Get JSON request
  2. Do something
  3. Produce JSON output with 'state' field which contains either code of a thrown exception or the "200" response. These state codes are quite useful for client which can distinguish different problems and react to them accordingly.

So there's a problem. System above produces code like this:

class UsernameFormatException extends InvalidArgumentException {
    public const USERNAME_FORMAT_UNDEFINED_CODE = "DM:USERCA:0001";

    public function __construct(string $message = "", string $calistoCode = self::USERNAME_FORMAT_UNDEFINED_CODE,
                                int $code = 0, \Throwable $previous = null) {
        parent::__construct($message, $calistoCode, $code, $previous);
    }
}

class PasswordFormatException extends InvalidArgumentException {
    public const PASSWORD_FORMAT_UNDEFINED_CODE = "DM:USERCA:0002";

    public function __construct(string $message = "", string $calistoCode = self::PASSWORD_FORMAT_UNDEFINED_CODE,
                                int $code = 0, \Throwable $previous = null) {
        parent::__construct($message, $calistoCode, $code, $previous);
    }
}

class InvalidLastActivityTimestampException extends InvalidArgumentException {
    public const INVALID_TIMESTAMP = "DM:USERCA:0003";

    public function __construct(string $message = "", string $calistoCode = self::INVALID_TIMESTAMP,
                                int $code = 0, \Throwable $previous = null) {
        parent::__construct($message, $calistoCode, $code, $previous);
    }
}

class InvalidCreationTimestampException extends InvalidArgumentException {
    public const INVALID_TIMESTAMP = "DM:USERCA:0004";

    public function __construct(string $message = "", string $calistoCode = self::INVALID_TIMESTAMP,
                                int $code = 0, \Throwable $previous = null) {
        parent::__construct($message, $calistoCode, $code, $previous);
    }
}

As seen, for each invalid-argument-case I create new CaliException-derived exception. I consider it as not cool. Is it good? How can I improve the code?

Bonus question: I read that I should throw \InvalidArgumentException when it's a programmer's mistake, so an exception must be not caught. But in my code there's no \InvalidArgumentException only my own version of this. Is it acceptable in the context of best practices and so on? And how to distinguish when it's a programmer's mistake and when it's mistake of user (invalid user input)? (After all, any invalid values passed to a function are invalid inputs relatively to this function)

\$\endgroup\$
0
\$\begingroup\$

With regard to if you need multiple exceptions or not:
I'd argue not in this particular situation, multiple exceptions are great for if you want to do different logic to handle them(so you plan to catch PasswordFormatException because that needs to be handled differently to your other validation errors).

To solve this you can add multiple constants on the same class if you want to return the code for what's invalid at the point you throw it:

class CaliException extends \Exception {
    const INVALID_USERNAME = "DM:USERCA:0001";
    const INVALID_PASSWORD = "DM:USERCA:0002";
    const INVALID_LAST_ACTIVITY_TIME = "DM:USERCA:0003";
    const INVALID_CREATION_TIME = "DM:USERCA:0004";

    private $caliCode;

    // Note that caliCode is now required, forcing you to pass in a constant
    // to say what the error is
    public function __construct(string $caliCode, string $message = "",
                                int $code = 0, Throwable $previous = null) {
        parent::__construct($message, $code, $previous);

        $this->caliCode = $caliCode;
    }

    function getCaliCode(): string {
        return $this->caliCode;
    }
}

// ....
// And then when you use it you now say it's a username or password field that failed validation

throw new CaliException(CaliException::INVALID_USERNAME, 'Username must not contain special characters');

As for if this should extend InvalidArgumentException:
I'd argue not, this feels more like you should have a class for UserInputValidationError or similar and extend this for your purpose. Failing validation of user input is not a fundamental problem with the program which InvalidArgumentException would imply but instead a natural consequence of having to handle user inputs.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.