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

I'm doing a really repetitive function here and I wondered how I might be able to simplify it. Any ideas? Here's the code: 
if ($file1 ne "") { push @tour_photos, ($file1); }
if ($file2 ne "") { push @tour_photos, ($file2); }
if ($file3 ne "") { push @tour_photos, ($file3); }
if ($file4 ne "") { push @tour_photos, ($file4); }
if ($file5 ne "") { push @tour_photos, ($file5); }
if ($file6 ne "") { push @tour_photos, ($file6); }
if ($file7 ne "") { push @tour_photos, ($file7); }
if ($file8 ne "") { push @tour_photos, ($file8); }
if ($file9 ne "") { push @tour_photos, ($file9); }
if ($file10 ne "") { push @tour_photos, ($file10); }

[download]
I've also got some huge lists of variables to accept from a cgi program and I wondered if there were an easier way to do this too: 
$address = $cgi->param('ADDRESS');
$bedrooms = $cgi->param('BEDROOMS');
$baths = $cgi->param('BATHS');
$garage = $cgi->param('GARAGE');
$roof = $cgi->param('ROOF');
$architectural_style = $cgi->param('ARCHITECTURAL_STYLE');
$floorplan = $cgi->param('FLOORPLAN');
$flooring = $cgi->param('FLOORING');
$heating = $cgi->param('HEATING');
$acreage = $cgi->param('ACREAGE');
$lot_dimensions = $cgi->param('LOT_DIMENSIONS');
$view = $cgi->param('VIEW');
$water_frontage = $cgi->param('WATER_FRONTAGE');
$short_description = $cgi->param('SHORT_DESCRIPTION');
$description = $cgi->param('DESCRIPTION');
$comments = $cgi->param('COMMENTS');
$area = $cgi->param('AREA');
$project = $cgi->param('PROJECT');
$zoning = $cgi->param('ZONING');
$taxes = $cgi->param('TAXES');
$tax_year = $cgi->param('TAX_YEAR');
$school_district = $cgi->param('SCHOOL_DISTRICT');
$schools = $cgi->param('SCHOOLS');
$mls = $cgi->param('MLS');
$price = $cgi->param('PRICE');
$downpayment = $cgi->param('DOWNPAYMENT');
$property_type = $cgi->param('PROPERTY_TYPE');
$thumbnail_photo = $cgi->param('THUMBNAIL_PHOTO');
$large_photo = $cgi->param('LARGE_PHOTO');
$tour_photos = $cgi->param('TOUR_PHOTOS');
$virtual_tour = $cgi->param('VIRTUAL_TOUR');
$virtual_tour_url = $cgi->param('VIRTUAL_TOUR_URL');
$flyer_url = $cgi->param('FLYER_URL');
$file1 = $cgi->param('file1');
$file2 = $cgi->param('file2');
$file3 = $cgi->param('file3');
$file4 = $cgi->param('file4');
$file5 = $cgi->param('file5');
$file6 = $cgi->param('file6');
$file7 = $cgi->param('file7');
$file8 = $cgi->param('file8');
$file9 = $cgi->param('file9');
$file10 = $cgi->param('file10');

[download]

Replies are listed 'Best First'.
Re: Simplifying the following code
by Fastolfe (Vicar) on Jan 14, 2001 at 00:29 UTC
    If you put all of your files into an array instead of an individual variable for each, you could iterate through your array. 
    <input name='file' ...>
<input name='file' ...>  <!-- instead of doing file1, file2 -->
<input name='file' ...>

my @files = $cgi->param('file');

foreach (@files) {
   # process as needed or just use as-is
}

    [download]
[reply]
[d/l]
Re: Simplifying the following code
by Kanji (Parson) on Jan 14, 2001 at 00:50 UTC

    Given the choice, Fastolfe's method is the better way to go, but you could do ...

    foreach my $file ( $file1, $file2, $file3, ... ) {
    push @tour_photos, $file 
        if defined($file) && $file ne '';
}

    [download]

    As for the second part of your question, it was asked and answered in Variable Variables, but why do all that assignment when you can use param('var')?

        --k.


[reply]
[d/l]
Re: Simplifying the following code
by stephen (Priest) on Jan 14, 2001 at 05:33 UTC
    Fastolfe is, as usual, correct about putting the files into a loop.

    To delve further into what Kanji was saying, you can avoid the object-oriented syntax (which is unnecessary here) by putting 

    use CGI qw/:cgi/;

    [download]
    at the top of your code. I'd avoid using all the temporary variables ('$virtual_tour' etc.) if I were you. There's no harm in using param('xxx') every time you want a variable, and every needless temporary variable adds to the confusion factor of your code ("Have I initialized $virtual_tour yet? Where?"). If you need to interpolate the variables into a string, you can use: 
    print STDOUT "The tax year is @{[ param('TAX_YEAR') ]}";

    [download]
    Or, better, 
    print STDOUT "The tax year is ", param('TAX_YEAR');

    [download]
    The only drawback of using 'param' all over the place is the possibility of misspelling some parameter names, which won't be detected by perl, even with '-w'. If this is a problem, you can define your parameter names as constants like so: 
    use constant DOWNPAYMENT => 'DOWNPAYMENT';

    [download]
    That might be overkill, but offers the advantage of having all of your parameter names in a single place. Then you can say: 
    print STDOUT "Downpayment is: ", param(DOWNPAYMENT);

    [download]
    but misspelling would give a compile-time error if you 'use strict' and -w (and you are, of course, yes? :) ) 
    # Gives an error
print STDOUT "Downpayment is: ", param(DOWNPAMENT);

    [download]
    Best of luck...

    stephen

[reply]
[d/l]
[select]
Re: Simplifying the following code
by extremely (Priest) on Jan 14, 2001 at 02:14 UTC
    Since you are using CGI you can do this and stick all your params in a single hash: 
    my %params = $cgi->Vars;
#  Now Kanji's code becomes
foreach my $file ( qw (file1 file2 file3 ...) ) {
  push @tour_photos, $params{$file}
    if exists($params{$file}) && $params{$file} ne '';
}

    [download]

    Update: Took the incorrect commas out of the qw(), *sigh* posting on weekends again... and changed 'defined' to 'exists' since it is now in a hash.

    Since it came up in chatterbox, I'd like to point out that CGI will take a multiple items with the same form 'name' and cram them into an array. Fastolfe demonstrated that in another post here. I should warn you that if the various "fileXX" files have specific purposes then that method may not be for you. Order from the client isn't guaranteed to be the same as the order in the HTML form. I can't recall a browser that fails to do so off the top of my head but that fact is out there, waiting to bite the overconfident. Like, for instance, me. =)

    Up-Update Kanji kindly points out that the Vars method of CGI happens to act like cgi-lib.pl and it puts multiple items into a single string separated by friggin' nulls "\00" so while the above is correct in warning you about disordered items, it is miserably wrong about how Vars treats your lists. =) Now, I'm going to crawl under a rock, thank you and good night.

    --
    $you = new YOU;
    honk() if $you->love(perl)

[reply]
[d/l]
Re: Simplifying the following code
by I0 (Priest) on Jan 14, 2001 at 18:16 UTC
    push @tour_photos, grep{$_ ne ''} map {${"file$_"}} 1..10;
    #but why do you have 10 separate variables instead of an array?

    ${lc $_}=$cgi->param($_) for $cgi->param;
    #but wouldn't it be simpler to use a single hash instead of huge lists of variables?
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom