https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502 --- Comment #27 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #25)
(In reply to Jonathan Druart from comment #23)
The classes of the exceptions should not be named with the module they are raised from.
Hmm. Just following what we already did (and approved). Like:
package Koha::Patron::Modification; use Koha::Exceptions::Patron::Modification; Koha::Exceptions::Patron::Modification::DuplicateVerificationToken->throw === package Koha::Exceptions::Patron::Modification;
'Koha::Exceptions::Patron::Modification::DuplicateVerificationToken' => { isa => 'Koha::Exceptions::Patron::Modification', description => "The verification token given already exists" }, ===
Please explain. Should we formulate a coding guideline here?
We might need to. I think the thing with the exceptions you add is: - There's no need to have them tied to output_pref, they could jus tbe top-level for the DateUtils namespace. Remember when you catch them, you have the context in which it happened and can build the needed actions. - The ones you add should be general exceptions: WrongParameters, MissingParameters, InvalidParamType. Keep in mind that you can use them like this: Koha::Exceptions::InvalidParamType->throw('DateTime object expected'); Koha::Exceptions::WrongParameters->throw('Conflicting parameters dt and str passed'); instead of baking a too specific exception. Regarding the Patron modification ones, I found that patron modification tokens were too specific, and they deserved its own exception. But it might be subject to future changes if someone raises it. -- You are receiving this mail because: You are watching all bug changes.