[Koha-devel] Bug 18539 - Forbid Koha::Objects->find calls in list context

[Julian Maurice]

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