Newbie Question: How do I make this code snippet a loop?

[prelim_questions]

I have only a couple weeks of experience with Perl. In that time, I
figured out how to put together a prototype CGI application. But at
times, I had to resort to writing bad code, because I did not know how
to do certain things in Perl.

How do I turn the following into a loop or otherwise make it much
easier to write?


if (param("pick") eq "$quiz11[4]"){
system("/home/username/script option1 option2 \"$quiz11[4]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[3]"){
system("/home/username/script option1 option2 \"$quiz11[3]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[2]"){
system("/home/username/script option1 option2 \"$quiz11[2]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[1]"){
system("/home/username/script option1 option2 \"$quiz11[1]\" ");
print redirect("http:my_url");
}


So in this example, $quiz11 has 4 elements, hence 4 conditionals and
in general would have number of conditionals equal to number of
elements. Also, within a given block of code like this, the URL, the
script, and the option1 and option2 do not change.

Thanks in advance... I am trying to learn as much as I can!

 

[Ben Morrow]

Quoth (e-mail address removed):

I have only a couple weeks of experience with Perl. In that time, I
figured out how to put together a prototype CGI application. But at
times, I had to resort to writing bad code, because I did not know how
to do certain things in Perl.

How do I turn the following into a loop or otherwise make it much
easier to write?

if (param("pick") eq "$quiz11[4]"){

You don't need the double quotes here. See perldoc -q quoting.

system("/home/username/script option1 option2 \"$quiz11[4]\" ");

I don't know what this script does, but if it's written in Perl there's
almost certainly a better way to use it than running it with system.

When you do need system, it's better to use the list form when you can:

system '/home/username/script', 'option1', 'option2', $quiz11[4];

or use qw// which splits on whitespace:

system qw{ /home/username/script option1 option2 }, $quiz11[4];

as it doesn't invoke the shell, so it's faster and you don't need the
double quotes (which aren't safe, by the way: what if $quiz11[4]
contained an '$'?).

print redirect("http:my_url");

} elsif (param("pick") eq "$quiz11[1]"){

Note that in Perl arrays start with element 0 (except when someone's
been messing with $[, which is a *very* bad idea). There are valid
reasons for skipping the first element, but it's usually more trouble
than it's worth.

So in this example, $quiz11 has 4 elements, hence 4 conditionals and
in general would have number of conditionals equal to number of
elements. Also, within a given block of code like this, the URL, the
script, and the option1 and option2 do not change.

In general the answer here is to use a hash. Build a hash with all valid
options as keys, and check it with exists. Something like

my %valid;
@valid{ @quiz11 } = ();

my $pick = param('pick');

if (exists $valid{ $pick }) {
system ..., $pick;
print redirect('http:...');
}

That second line does quite a lot:

Take a list of all the allowed values,

@quiz11

look each of them up in the hash %valid and return a list of the
results (none of them will exist yet, of course, but that doesn't
matter)

@valid{ @quiz11 }

and assign the empty list to the result.

@valid{ @quiz11 } = ();

This creates all the corresponding entries in the hash (so exists says
they exist) without assigning them any value (since we don't need to).
If you find it less confusing you can write something like

$valid{$_} = 1 for @quiz11;

which loops through the allowed values and sets the correspoding entry
in the hash to 1, but the way I used is more efficient. This often
doesn't matter, though, so use whichever you find clearer.

If you don't make other use of @quiz11, you can keep the list in a hash
to start with. If necessary you can retrieve the whole list with keys
%quiz11.

Thanks in advance... I am trying to learn as much as I can!

Good luck .

Ben

 

[Jürgen Exner]

How do I turn the following into a loop or otherwise make it much
easier to write?
if (param("pick") eq "$quiz11[4]"){
system("/home/username/script option1 option2 \"$quiz11[4]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[3]"){
system("/home/username/script option1 option2 \"$quiz11[3]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[2]"){
system("/home/username/script option1 option2 \"$quiz11[2]\" ");
print redirect("http:my_url");
} elsif (param("pick") eq "$quiz11[1]"){
system("/home/username/script option1 option2 \"$quiz11[1]\" ");
print redirect("http:my_url");
}

Easy. Just check what changes between each case and make that part a
variable:

for (1..4) {
if (param("pick") eq $quiz11[$_]){
system("/home/username/script option1 option2 \"$quiz11[$_]\" ");
print redirect("http:my_url");
}
}

However, there are probably better ways to achive the same end goal. E.g. I
would probably create a hash mapping from those @quiz11 values to the
desired print values directly. Then you don't need dozens of comparisons and
the whole loop would collaps to a simple

system('/home/username/script option1 option2 "'.
$mappingtable{param('pick')} .
'"');

jue