https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19809
Marcel de Rooy <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #13 from Marcel de Rooy <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
