https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24254 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion Keywords|release-notes-needed | --- Comment #19 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #18)
(In reply to Jonathan Druart from comment #17)
(In reply to Tomás Cohen Arazi from comment #15)
(In reply to Jonathan Druart from comment #13)
Why does the method take rules in parameter? Why don't we simply build the rules from the pref in the method?
If it was called in a loop, we could reuse the rules. That was the idea. Maybe we should read the rules locally if the parameter was not passed at all (i.e. !exists $params->{rules}).
What for? Performance? If we are looping on biblios then it's not that reading the pref and building the rules that will have an impact on perf. If you are concerned about that I would cache it at package level (->{_item_hide_rules}).
I think it's better to prevent calls that will forget to pass the rules, or having to update all the callers if we decide to add one rules.
What do you think?
I usually prefer explicit vs. implicit. But not a strong position on this particular case. The 'if passed use it, if not, read it' approach seems to me like the best compromise option. This could be a follow-up bug (it requires new tests, probably adapt the callers)
I don't think it's explicit vs implicit. ->filter_by_visible_in_opac is explicit already. If you are passing a set of rules then it would be ->filter_by_rules As I said I am also concerned about the need to update the callers if rules are added. If we agree on that it should be done on this bug report, not a follow-up bug. -- You are receiving this mail because: You are watching all bug changes.