On Wed, 28 Jul 2021 at 03:52, Dagfinn Ilmari Mannsåker <ilm...@ilmari.org> wrote: > > Alvaro Herrera <alvhe...@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