Junio C Hamano wrote: > Jakub Narebski <jnareb@xxxxxxxxx> writes: > >> 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"; That's better. I didn't know about this trick. Patch will follow. BTW. I have encountered this error in true situation, while doing history pagination because I forgot in parse_difftree_raw_line that the map(function, list) works only for prototyped functions, and had undef fed to href. >> 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). So perhaps my %mapping = @mapping; my @params_keys_sorted; for (my $i = 0; $i < @mapping; $i += 2) { if (exists $params{$mapping[$i]}) { push @params_keys_sorted, $mapping[$i]; } } This way we have only one loop over all valid parameter names, and we wouldn't need grep nor conditional expression in map, and map would loop over all (valid) params keys only. -- Jakub Narebski Warsaw, Poland ShadeHawk on #git - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html