[Koha-bugs] [Bug 19809] Koha::Objects::find calls do not need to be forbidden in list context
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Tue Nov 5 16:11:32 CET 2019
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19809
Marcel de Rooy <m.de.rooy at rijksmuseum.nl> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|Signed Off |Failed QA
--- Comment #13 from Marcel de Rooy <m.de.rooy at 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.
More information about the Koha-bugs
mailing list