tphyahoo has asked for the wisdom of the Perl Monks concerning the following question:

Hello monks. The below code runs ok, does what it supposed to do. I feel it has a lot of repetition though, and was wondering if it could be written better, or more elegantly, or said otherwise, written shorter, with less repetition. Thanks for any guidance! 
    my %fields;
    #$csvIn is a Text::xSV object.
    while ($csvIn->get_row()) {

        ($fields{partner},
        $fields{kampagne},
        $fields{keywordCluster},
        $fields{keyword},
        $fields{clicks},
        $fields{leads},
        $fields{orders},
        $fields{jaronVerguetung},
        $fields{partnerVerguetung},
        $fields{profit}) 
        = $csvIn->extract(qw(partner
            kampagne
            keywordCluster
            keyword
            clicks
            leads
            orders
            jaronVerguetung
            partnerVerguetung
            profit));

        #set nulls to empty string.  Have to do this or there will be 
+problems later printing the data rows.
        (($fields{partner},
        $fields{kampagne},
        $fields{keywordCluster},
        $fields{keyword},
        $fields{clicks},
        $fields{leads},
        $fields{orders},
        $fields{jaronVerguetung},
        $fields{partnerVerguetung},
        $fields{profit}) ) = map { unless ( defined ($_) ) { "" } else
+ {$_} }     
                    ($fields{partner},
                    $fields{kampagne},
                    $fields{keywordCluster},
                    $fields{keyword},
                    $fields{clicks},
                    $fields{leads},
                    $fields{orders},
                    $fields{jaronVerguetung},
                    $fields{partnerVerguetung},
                    $fields{profit} );
    }

[download]
UPDATE: Thanks very much. Will work on rewriting and repost. Wish I could vote you all up but I have squandered all my xp for the day.

s/until later/bis bald/

:)

Replies are listed 'Best First'.
Re: Text::xSV question - could this be written better?
by Animator (Hermit) on May 20, 2005 at 13:44 UTC

    Instead of listing the individuals values you could use a hash slice:

    Something like: @fields{qw/partner kampagne keywordCluster keyword ..../}

    And maybe it would be better to use another construct then 'map'... 'for' can be useful, since it alias the element which would mean you need to write $fields{partner} only once instead of twice (when setting the default value).

    Something like: 

    for (@fields{qw/partner kampagne .../}) { $_ = "", unless (defined $_)
+; }

    [download]

    Updated, added second code-example

    Update2: instead of listing each element in the for you could also use values %fields. This will ensure that there are no undefS in the hash

[reply]
[d/l]
[select]

      Exactly! A hash slice, but i would also use an array for the field names: 

      # untested
my @fields = qw(
    partner
    kampagne
    keywordCluster
    keyword
    clicks
    leads
    orders
    jaronVerguetung
    partnerVerguetung
    profit
);

@fields{@fields} = $csvIn->extract(@fields);

      [download]

      jeffa

      L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)
[reply]
[d/l]
[select]
Re: Text::xSV question - could this be written better?
by davidrw (Prior) on May 20, 2005 at 13:51 UTC
    You can use a hash slice for the first part, and just a foreach loop for the second part, but the key is to just store the key names so they aren't repeated -- also you now just have a single place to make changes in, so much easier to maintain. 
        my %fields;
    #$csvIn is a Text::xSV object.
    my @field_names = qw/ partner kampagne keywordCluster keyword clic
+ks leads
        orders jaronVerguetung partnerVerguetung profit /;
    while ($csvIn->get_row()) {
        @fields{@field_names} = $csvIn->extract( @field_names );
        foreach my $k ( @field_names ){
          $fields{$k} = "" unless defined $fields{$k};
        }
    }

    [download]
    Update: changed @k to @field_names to be a little more descriptive.
[reply]
[d/l]
      Yes davidrw, very nice indeed, I just dropped it in and it worked. Only I wound up using @field_names instead of @k. Any particular (idiomatic) reason for that variable name?

      Now, going back to bone up on hash slices... which I was already doing when other stuff came in to distract me... Is this a hash slice?

      Thanks again.

[reply]
Re: Text::xSV question - could this be written better?
by tilly (Archbishop) on May 20, 2005 at 15:33 UTC 
    #$csvIn is a Text::xSV object.
while ($csvIn->get_row()) {
  my %fields = $csvIn->extract_hash();

  # time passes

  # set nulls to empty string.  Have to do this or there will
  # be problems later printing the data rows.
  defined($_) or $_ = "" for values %fields;
}

    [download]
    The trick for setting nulls to "" might be too cute though. It relies on the way that Perl uses aliasing in a way that might puzzle whoever maintains the code. You might therefore prefer the more verbose: 
      for (keys %fields) {
    unless (defined($fields{$_})) {
      $fields{$_} = "";
    }
  }

    [download]
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom