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

Discussion in 'Perl Misc' started by prelim_questions@yahoo.com, Dec 1, 2007.

  1. Guest

    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!
     
    , Dec 1, 2007
    #1
    1. Advertising

  2. Ben Morrow Guest

    Quoth :
    > 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");

    <snip>

    > } 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
     
    Ben Morrow, Dec 1, 2007
    #2
    1. Advertising

  3. wrote:
    > 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
     
    Jürgen Exner, Dec 1, 2007
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Hedr
    Replies:
    1
    Views:
    564
    Raymond Hettinger
    Jun 25, 2003
  2. KK
    Replies:
    14
    Views:
    606
    Bo Persson
    Jun 30, 2006
  3. Replies:
    12
    Views:
    524
    Daniel T.
    Dec 4, 2007
  4. Diwa
    Replies:
    0
    Views:
    995
  5. Isaac Won
    Replies:
    9
    Views:
    387
    Ulrich Eckhardt
    Mar 4, 2013
Loading...

Share This Page