https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25039 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #7)
(In reply to Jonathan Druart from comment #2)
+ else { + Koha::Exceptions::WrongParameter->throw( + "'days' must be an integer" + ) unless looks_like_number($days); + $due_date->add( days => $days );
Developers will get an error anyway if $days is not an integer (DateTime will raise its own meaningful exception), or $hard_due_date is not a DateTime.
I tried removing the checks, and the results are not cool:
- Removing the check for DateTime object => found: Can't locate object method "clone" via package "cat" (perhaps you forgot to load "cat"?) at /kohadevbox/koha/Koha/Checkout.pm line 190.
Yes, that's the default perl error, everybody knows what it means :)
- Removing the check for number => found: normal exit, and a simple warning: Argument "dog" isn't numeric in addition (+) at /usr/lib/x86_64-linux-gnu/perl5/5.24/DateTime/Duration.pm line 74.
So, from the developer POV, in this particular cases, I still think my patch is better with the checks than not having them. I agree on something: It is boring to code this much for a small feature. But I would do it like this anyway.
There is a philosophic change we are making in these patches, and if you want to generalize them I think the discussion should be brought to the dev team. I guess there are hundreds of places where this check would be needed if such idea is adopted. What about splitting the patches in 2 parts, one for the move and the tests, another one moved to its own bug report for the data type check? -- You are receiving this mail because: You are watching all bug changes.