[Koha-bugs] [Bug 18677] issue_id is not added to accountlines for lost item fees

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

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

--- Comment #17 from M. Tompsett <mtompset at hotmail.com> ---
(In reply to Jonathan Druart from comment #16)
> There is only 1 call, from C4::Circulation::LostItem, the occurrence in
> C4::Overdues is part of a comment

So it is. Still, I thought the general thrust of development was to move
towards the Koha namespace, and if not, perhaps hashref the parameter list, so
that at least it gets closer to our coding guidelines.
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_should_be_used_as_arguments

Also, that comment in C4::Overdues makes me wonder about a tiny refactor / move
into the Koha name space. But perhaps that is beyond the scope of this.

Changing the parameter to a hashref should be a simple enough fix. Though, I do
think putting the lookup in the function, rather than in the calling function,
and thus avoid the need to change the parameter list, is probably better. Less
files affected. Less is more.

It's not like chargelostitems is called for the same thing multiple times, is
it? If it is, the external look up and hashref parameter is probably preferred.
After all, we don't want to slow things down either.

To summarize: if there are no speed issues caused by a double call, then moving
the look up into the function is the preferred fix. If there is a speed issue,
then hashref'ing the parameter list and leaving it external is preferred.

Just my take on it.

-- 
You are receiving this mail because:
You are watching all bug changes.