[Koha-bugs] [Bug 25039] Move new due calculation to Koha::Checkout

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

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

--- Comment #8 from Jonathan Druart <jonathan.druart at 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.