Re: [RFD] gitweb: href() function to generate URLs for CGI

From: Junio C Hamano <junkio@cox.net>
Date: Aug 22, 2006, 07:09 UTC
Message-ID: <7vlkphqmac.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <eccujr$90h$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

Show 11 quoted lines

> This doesn't work as expected. Map works on _every_ element of list; if
> expression doesn't return anything it just puts undef I think. For example
> while
>
>         print join(";", 
>                 map { 
>                         "a/$_" if ($_ eq lc($_)) 
>                 } ("a", "b", "C", "d")),
>                 "\n";'
>
> returns "a/a;a/b;;a/d" (notice the doubled ';'), the correct way would be to

... say this, than using extra grep {}.

        print join(";", 
                map { 
                        ($_ eq lc($_)) ? "a/$_" : ()
                } ("a", "b", "C", "d")),
                "\n";

Show 21 quoted lines

> What about my proposed solution? Don't sort, use sorted keys instead, i.e.
> something like (after unmangling whitespace)
>
> sub href(%) {
>         my @mapping = ( project => "p",
>                         action => "a",
>                         hash => "h",
>                         hash_parent => "hp",
>                         hash_base => "hb",
>                         file_name => "f",
>                         file_parent => "fp",
>                         page => "pg",
>                         searchtext => "s",
>                         );
>         my %mapping;
>         my @mapping_keys;                        
>         for (my $i = 0; $i < @mapping; $i += 2) {
>                 my ($k, $v) = ($mapping[$i], $mapping[$i+1]);
>                 $mapping{$k} = $v;
>                 push @mapping_keys, $k;
>         }

Now this loop got an expensive way to say %mapping = @mapping plus assigning @mapping_keys, so I would do only the push part in the loop for readability while changing the name of the variable a bit more meaningful.

	my %mapping = @mapping;
        my @valid_params;
	for (my $i = 0; $i < @mapping; $i += 2) {
	        push @valid_params, $mapping[$i];
	}

But aside from that, I very much prefer your version that loops over what _we_ define (i.e. @valid_params), rather than what the user happens to throw at us (i.e. keys %params).