--- Teresa Raymond <[EMAIL PROTECTED]> wrote:
> Any thoughts on how to make this code more succinct?
There are a variety of things that could be done to clear this up. I'll just focus on
the concept
of "not repeating yourself".
Consider the following 60 lines from your code:
> if ($location=~/East/ig)
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="East " checked>East<br>
> PrintTag
> }
> else
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="East ">East<br>
> PrintTag
> }
> if ($location=~/West/ig)
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="West " checked>West<br>
> PrintTag
> }
> else
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="West ">West<br>
> PrintTag
> }
> if ($location=~/North/ig)
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="North " checked>North<br>
> PrintTag
> }
> else
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="North ">North<br>
> PrintTag
> }
> if ($location=~/Southeast/ig)
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="Southeast " checked>Southeast<br>
> PrintTag
> }
> else
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="Southeast ">Southeast<br>
> PrintTag
> }
> if ($location=~/Southwest/ig)
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="Southwest " checked>Southwest
> PrintTag
> }
> else
> {print <<"PrintTag";
> <input type="checkbox" name="location" size="20" maxlength="20"
> value="Southwest ">Southwest
> PrintTag
> }
Notice that all of those if/else statements are *identical* except for the compass
direction. You
can stuff those in an array, iterate over it, and have much more succinct code that's
easier to
maintain:
my @locations = qw/ East West North Southeast Southwest /;
foreach my $array_location ( @locations ) {
my $checked = '';
if ( $location =~ /$array_location/ig ) {
$checked = ' checked';
}
print qq/<input type="checkbox" name="location" size="20" maxlength="20"
value="$array_location
"$checked>$array_location<br>\n/;
Note that we've reduced 60 lines of code to 7, with the same output! Whenever you see
yourself
writing repetitive code (or cutting and pasting a lot), start looking for common
elements and look
for ways to abstract them out like this.
Cheers,
Curtis Poe
=====
Senior Programmer
Onsite! Technology (http://www.onsitetech.com/)
"Ovid" on http://www.perlmonks.org/
__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail - only $35
a year! http://personal.mail.yahoo.com/
