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

I am still a beginner to Perl language and to handle one of
the situation I am doing three different foreach for
different condition. I am just wondering if this can be
achieve or done in musch (better/elegant) way? or if more experienced programmer has to do the same thing how he/she would do it?

foreach my $textField qw(fullname username authname email)
    {
        $template->param($textField, $self->param($textField) ||
                                     $userData->{$textField} || '');

        $finalData{$textField}  = $self->param($textField) ||
                                  $userData->{$textField} || '';
    }

    foreach my $checkBox qw(chk1 chk2 chk3)
    {
        if ($self->param($checkBox) eq '1' || $userData->{$checkBox} e
+q '1')
        {
            $template->param($checkBox, 'checked');
            $finalData{$checkBox} = 1;
        }
    }

    if($which eq 'INTERNAL')
    {
        foreach my $fixField qw(emp_id emp_name emp_cat)
        {
            $template->param($fixField, $self->param($fixField) ||
                                         $userData->{$fixField} || '')
+;
        }
   }

[download]
Thanks!

Replies are listed 'Best First'.
Re: Beginner's question about elegent foreach
by CountZero (Bishop) on Feb 04, 2011 at 07:01 UTC
    You have two loops which unconditionally do different things to different variables/fields and one loop which conditionally does similar things as the first loop but to different variables/fields.

    Using three separate loops for this is OK.

    CountZero

    A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James

[reply]
Re: Beginner's question about elegent foreach
by bellaire (Hermit) on Feb 04, 2011 at 14:21 UTC
    For what you've got now, it's just fine. There are a couple of small changes you could make. In the first loop, many common classes that have a param method will let you do this to reduce duplication: 
        $template->param($textField, $self->param($textField) ||
                                  $userData->{$textField} || '');
    $finalData{$textField} = $template->param($textField);

    [download]
    You could also move the third loop next to the first, since they are doing similar things, if the order isn't important. Another technique you could use, since you're curious, is a dispatch table. If you end up having a lot of kinds of these data handlers, or if they get quite large, this can be advantageous. Although it's probably overkill for only 3 different cases, here's what that would look like: 
    my %fieldHandler = (
    textField => sub 
    {
        my $field = shift;
        $template->param($field, $self->param($field) || $userData->{$
+field} || '');
        $finalData->{$field} = $template->param($field);
    },
    checkBox => sub 
    {
        my $field = shift;
        if ($self->param($field) eq '1' || $userData->{$field} eq '1')
+ 
        {
            $template->param($field,'checked');
            $finalData{$field} = 1;
        }
    },
    fixField => sub 
    {
        my $field = shift;
        if ($which eq 'INTERNAL') 
        {
            $template->param($field, $self->param($field) || $userData
+->{$field} || '') 
        }
    },
);

my %fieldMap = (
    fullname => 'textField',
    username => 'textField',
    authname => 'textField',
    email    => 'textField',
    chk1     => 'checkBox',
    chk2     => 'checkBox',
    chk3     => 'checkBox',
    emp_id   => 'fixField',
    emp_name => 'fixField',
    emp_cat  => 'fixField',
);

foreach my $fieldName (keys %fieldMap)
{
    my $handler = $fieldMap{$fieldName};
    $fieldHandler{$handler}->($field);
}

    [download]
    If the order in which you process the fields is important, you'd have to specify it separately: 
    my @fieldNames = qw(fullname username authname email chk1 chk2 chk3 em
+p_id emp_name emp_cat);

foreach my $fieldName (@fieldNames)
...

    [download]

    This has the advantage of putting all of your field names close together with their handler mappings. It also gives you the option of moving off very complex handlers into named subroutines instead of anonymous ones, but if you do that you'll have to pass along references to the data you're using. (This example only works because the anonymous subroutines are acting as closures, and have access to variables in the same lexical scope in which they were defined.)

    As stated, for only three types of field this is probably not necessary, but there more you have and the more complex your data handling becomes, the more spread out and difficult to follow your existing structure will become. Eventually all of those foreach loops start to look the same, making it harder to maintain.

[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom