CODEWITHSUNDEEP
perldictionaryforeachgrep
user2993033
user2993033

Reputation: 31

Can I combine this Perl in to a single map-grep chain?

Can I combine this Perl in to a single map-grep chain? A little voice says I should be able to, but I don't know how!

# expand sponsor keys in to a list of sponsor objects.
foreach my $event (@events) {
  next unless exists $event->{sponsors} && ! ref $event->{sponsors};
  $event->{sponsors} =
    [ map { $lookup{$_} }
        grep { exists $lookup{$_} }
          split( /\s*,\s*/, $event->{sponsors} ) ];
}

Upvotes: 3

Views: 956

Answers (3)

ThisSuitIsBlackNot
ThisSuitIsBlackNot

Reputation: 24073

Personally, I think a simple foreach would be much more readable than using map/grep:

foreach my $event (@events) {
    next unless exists $event->{sponsors} && ! ref $event->{sponsors};

    my @sponsors;
    foreach my $sponsor ( split /\s*,\s*/, $event->{sponsors} ) {
        push @sponsors, $sponsor if exists $lookup{$sponsor};
    }

    $event->{sponsors} = [ @sponsors ];
}

Upvotes: 1

Axeman
Axeman

Reputation: 29854

I'm a big fan of (readable) map-grep chains. Despite that, although I know map increasingly handles being used in a void context better, I don't like to use map in a void context. Added to that, I prefer hash slices to grepping something in an array. Provided that the value for lookup is not undef, grepping all the defined values from a hash slice ought to work just as well as grep-ing for the ones that exist and then transforming them into the lookup value.

So I offer this, map solution: 

map { 
    $_->{sponsors}
        = [ grep {; defined } @lookup{ split( /\s*,\s*/, $_->{sponsors} ) } ]
        ;
}
grep { $_->{sponsors} && !ref $_->{sponsors} }  @events
;

The problem with using map here, is that you're not creating one list of items from another. You are updating one list.

As far as my preferences go, you could just as easily do a do-foreach as well: 

 do { 
    $_->{sponsors}
        = [ grep {; defined } @lookup{ split( /\s*,\s*/, $_->{sponsors} ) } ]
        ;
 }
 foreach { $_->{sponsors} && !ref $_->{sponsors} }  @events
;

But that's not that different from your original. And you could just as easily put that foreach on the top. But then you have a loop, and I have no problem with the next there either. I think the biggest gain that can be made is with the hash slices instead of grepping on and mapping to the same value.

Upvotes: 3

Tudor Constantin
Tudor Constantin

Reputation: 26861

haven't tried it, but it should be along the lines of:

 my $event;
 push( @{ $event->{sponsors} }, 
         map { $lookup{$_} }
           grep { $lookup{$_} }
             split( /\s*,\s*/, $event->{sponsors} ) 
               grep { $_->{sponsors} && $event = $_ } @events);

Upvotes: 0

Related Questions