[Koha-devel] Bug 18539 - Forbid Koha::Objects->find calls in list context
David Cook
dcook at prosentient.com.au
Thu Dec 14 00:51:30 CET 2017
Alternatively, you could write:
my $foo = Koha::Patrons->find("foo");
my $hashref = { a => $foo, b => "bar" };
or
my $foo = Koha::Patrons->find("foo");
my $hashref = { b => "bar" };
$hashref->{a} = $foo if $foo;
Personally, I'm not a fan of putting function calls in data structures.
David Cook
Systems Librarian
Prosentient Systems
72/330 Wattle St
Ultimo, NSW 2007
Australia
Office: 02 9212 0899
Direct: 02 8005 0595
-----Original Message-----
From: koha-devel-bounces at lists.koha-community.org
[mailto:koha-devel-bounces at lists.koha-community.org] On Behalf Of Julian
Maurice
Sent: Thursday, 14 December 2017 1:57 AM
To: Marcel de Rooy <M.de.Rooy at rijksmuseum.nl>
Cc: koha-devel at lists.koha-community.org
Subject: Re: [Koha-devel] Bug 18539 - Forbid Koha::Objects->find calls in
list context
Probably, but that's not my point.
I understand why the previous code needed to be fixed, which is to avoid
that
{ a => Koha::Patrons->find("foo"), b => "bar" }
would result in { a => "b", "bar" => undef }
My question is: why the solution to this problem was to forbid calls in list
context while we could simply return an explicit undef?
It forces us to write 'scalar' in front of a call that should always return
a single value, and in my opinion it's a "code smell"
Le 13/12/2017 à 15:29, Marcel de Rooy a écrit :
> i forgot to add scalar in the second..
>
> probably you should
>
> if( $a = xxx->find )
>
> push @b, $a
>
>
> ----------------------------------------------------------------------
> --
> *Van:* Julian Maurice <julian.maurice at biblibre.com>
> *Verzonden:* woensdag 13 december 2017 15:20:34
> *Aan:* Marcel de Rooy
> *Onderwerp:* Re: [Koha-devel] Bug 18539 - Forbid Koha::Objects->find
> calls in list context
>
> This particular line will work (only the first, in the second, find is
> still called in list context), but @a would contain one element
> (undef) and the example code given by Jonathan will also fail, whether
> we call find in list context or not.
>
> Le 13/12/2017 à 15:12, Marcel de Rooy a écrit :
>> Are you sure ?
>>
>> push @a, scalar Koha..->find
>>
>> or @a = ( ...->find )
>>
>>
>> should work imo
>>
>> ---------------------------------------------------------------------
>> ---
>> *Van:* koha-devel-bounces at lists.koha-community.org
>> <koha-devel-bounces at lists.koha-community.org> namens Julian Maurice
>> <julian.maurice at biblibre.com>
>> *Verzonden:* woensdag 13 december 2017 15:07:11
>> *Aan:* Jonathan Druart
>> *CC:* koha-devel at lists.koha-community.org
>> *Onderwerp:* Re: [Koha-devel] Bug 18539 - Forbid Koha::Objects->find
>> calls in list context
>>
>> Ok, but the proposed fix doesn't fix this code:
>>
>> @a = scalar Koha::Patrons->find('foo'); if (@a) { # still equals to 1
>> say $a[0]->borrowernumber; # still BOOM }
>>
>> And yes, that was the link ;)
>>
>> Le 13/12/2017 à 14:58, Jonathan Druart a écrit :
>>> Hi Julian,
>>>
>>>
>>>> @a = Koha::Patrons->find('foo'); # would result in @a = (undef)
>>>
>>> And that leads to issues:
>>> if ( @a ) { # = 1
>>> say $a[0]->borrowernumber; # BOOM
>>> }
>>>
>>> See also https://perlmaven.com/how-to-return-undef-from-a-function
>>> (was it the link you were talking about?)
>>>
>>> Cheers,
>>> Jonathan
>>>
>>> On Wed, 13 Dec 2017 at 10:34 Julian Maurice
>>> <julian.maurice at biblibre.com <mailto:julian.maurice at biblibre.com>>
wrote:
>>>
>>> Hi developers,
>>>
>>> I stumbled upon a line of code recently and I can't figure out
>>>why it
>>> has be done this way. I hope you can help me :)
>>>
>>> The line in question is in Koha::Objects::find:
>>>
>>> croak 'Cannot use "->find" in list context' if wantarray;
>>>
>>> I read the two bugs (18539 and 18179) and the link given by
>>>Jonathan but
>>> I still don't understand why the call in list context has been
>>> forbidden. Why not simply return undef (an explicit undef) when
>>>no
>>> records have be found ? It would work as expected in scalar and
>>>list
>>> contexts.
>>>
>>> Here is a possible rewrite of 'find' to better explain what I mean:
>>>
>>> sub find {
>>> my ( $self, @pars ) = @_;
>>>
>>> my $object = undef;
>>>
>>> @pars = grep { defined } @pars;
>>> if (@pars) {
>>> my $result = $self->_resultset()->find(@pars);
>>> if ($result) {
>>> $object =
>>> $self->object_class()->_new_from_dbic($result);
>>> }
>>> }
>>>
>>> return $object;
>>> }
>>>
>>> @a = Koha::Patrons->find('foo'); # would result in @a = (undef)
>>> {a => K::P->find('foo'), b => 'bar'}; # would result in {a =>
>>>undef, b
>>> => 'bar'}
>>>
>>> Please tell me what you think.
>>>
>>> --
>>> Julian Maurice <julian.maurice at biblibre.com
>>> <mailto:julian.maurice at biblibre.com>>
>>> BibLibre
>>> _______________________________________________
>>> Koha-devel mailing list
>>> Koha-devel at lists.koha-community.org
>>> <mailto:Koha-devel at lists.koha-community.org>
>>>
>>>http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
>>> website : http://www.koha-community.org/
>>> git : http://git.koha-community.org/
>>> bugs : http://bugs.koha-community.org/
>>>
>>
>> --
>> Julian Maurice <julian.maurice at biblibre.com> BibLibre
>> _______________________________________________
>> Koha-devel mailing list
>> Koha-devel at lists.koha-community.org
>> http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
>> website : http://www.koha-community.org/ git :
>> http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
>
> --
> Julian Maurice <julian.maurice at biblibre.com> BibLibre
--
Julian Maurice <julian.maurice at biblibre.com> BibLibre
_______________________________________________
Koha-devel mailing list
Koha-devel at lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : http://www.koha-community.org/ git :
http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
More information about the Koha-devel
mailing list