Code Review Asked on December 12, 2021
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:
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)
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.
Answered by scragar on December 12, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP