On 7/27/21 11:01 AM, Alvaro Herrera wrote:
> On 2021-Jul-28, David Rowley wrote:
>
>> 0003: Is a tidy up patch to make the 'includes' field an array rather
>> than a string
> In this one, you can avoid turning one line into four with map,
>
> - $p->AddIncludeDir($pl_proj->{includes});
> + foreach my $inc (@{ $pl_proj->{includes} })
> + {
> + $p->AddIncludeDir($inc);
> + }
>
> Instead of that you can do something like this:
>
> + map { $p->AddIncludeDir($_); } @{$pl_proj->{includes}};
using map() for a side effect like this is generally frowned upon. See
<https://metacpan.org/pod/Perl::Critic::Policy::BuiltinFunctions::ProhibitVoidMap>
do { $p->AddIncludeDir($_); } foreach @{$pl_proj->{includes}};
would be an ok one-liner.
>
>> 0004: Adds code to check for duplicate references and libraries before
>> adding new ones of the same name to the project.
> I think using the return value of grep as a boolean is confusing. It
> seems more legible to compare to 0. So instead of this:
>
> + if (! grep { $_ eq $ref} @{ $self->{references} })
> + {
> + push @{ $self->{references} }, $ref;
> + }
>
> use something like:
>
> + if (grep { $_ eq $ref} @{ $self->{references} } == 0)
>
But I believe that's a widely used idiom :-)
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
