[Koha-bugs] [Bug 17502] Add type check to output_pref and use exceptions

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17502

--- Comment #27 from Tomás Cohen Arazi <tomascohen at 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.