On Wed, 28 Jul 2021 at 03:52, Dagfinn Ilmari Mannsåker
<ilmari@ilmari.org> wrote:
>
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>
> > 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}};
>
> map (and grep) should never be used void context for side effects. Our
> perlcritic policy doesn't currently forbid that, but it should (and
> there is one violation of that in contrib/intarray). I'll submit a
> patch for that separately.
>
> The acceptable one-liner version would be a postfix for loop:
>
> $p->AddIncludeDir($_) for @{$pl_proj->{includes}};
I'm not sure if this is all just getting overly smart about it.
There's already a loop next to this doing:
foreach my $type_lib (@{ $type_proj->{libraries} })
{
$p->AddLibrary($type_lib);
}
I don't object to changing mine, if that's what people think who are
more familiar with Perl than I am, but I do think consistency is a
good thing. TBH, I kinda prefer the multi-line loop. I think most
people that look at these scripts are going to be primarily C coders,
so assuming each of the variations do the same job, then I'd rather
see us stick to the most C like version.
In the meantime, I'll just change it to $p->AddIncludeDir($_) for
@{$pl_proj->{includes}};. I just wanted to note my thoughts.
David
