https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19809 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Julian Maurice from comment #12)
(In reply to Marcel de Rooy from comment #11)
(In reply to Martin Renvoize from comment #10)
This seems sane to me and I like to follow the example set by DBIx::Class to be honest.. that project has been around for a long time and has made allot of good decisions for good reasons.
I would agree with you if we were writing Koha::Objects->find at this moment. But in the meantime they go back to bug 13019, pushed 02/2015. How many find calls do we have now, and associated if conditions etc.? In view of that volume and the risk of breaking a lot of code, I would not recommend it now.
Currently, calls to `find` are always in scalar context (because otherwise Koha croaks), and this patch doesn't change the behaviour of `find` in scalar context. Why do you think there's a risk of breaking code ?
Hi Julian, This might have been a bit overcautious, you are right. But looking at the code, I still see some problems: + @pars = grep { defined } @pars; This is not the same as the earlier check: - return if !@pars || none { defined($_) } @pars; Btw note that this statement would return an empty list (with the croak removed). return $object; This is no longer good. Since you are returning 'undef' here literally while you want to return empty list if it is list context. You should do: if (@pars) { my $result = $self->_resultset()->find(@pars); if ($result) { $object = $self->object_class()->_new_from_dbic($result); return $object; } } return; The last return gives empty list in list context and undef in scalar context. I would like to see that tested too in the Objects.t test. And we need a rebase on members/pay.pl -- You are receiving this mail because: You are watching all bug changes.