Re: Reduce the number of special cases to build contrib modules on windows

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