Beefy Boxes and Bandwidth Generously Provided by pair Networks
Pathologically Eclectic Rubbish Lister
 
PerlMonks  

a question about working with the forms and array

by Anonymous Monk
on Oct 09, 2003 at 16:02 UTC ( [id://297962]=perlquestion: print w/replies, xml ) Need Help??

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

Greetings,

I have been working on a small script for learning purposes and had a question about working with a array.

I am currently able to open a dir, read in the contents close the dir and foreach $_ array them and print $_ . I am using a small form hardcoded into the script that simply adds a checkbox next to each value in the array of files found. I then select which items/values I want to work with and submit the form.(it then opens a txt file and should place the selected values in it. Can someone please help me find my error, I am so close and always interested in learning more...

Thank you very much for your time~
Jim

#!/usr/bin/perl use CGI::Carp qw(fatalsToBrowser); ######################## $path="/home/foo/www/dir"; $location="http://mysite.com/dir"; print "Content-type: text/html\n\n"; ######################## &Begin; sub top { opendir(DIR, "$path" ); @thefiles = sort(grep(/html$/, readdir(DIR))); $count = 0; foreach $_ ( sort @thefiles) { $count++; } closedir(DIR); print "<html>"; print "<head><title>MY SIMPLE FILE SELECTOR</title></head>"; print "<body bgcolor =\"#ffffff\" text =\"#000000\" link = \"#000000\" + vlink = \"#000000\" alink = \"#000000\">"; print "<center><h2>MY SIMPLE FILE SELECTOR</h2>"; print "<br><br><small><b>The following [$count] files were found in th +e [$path] Dir.</b></small><br><br>"; print "</center><table>"; } sub Begin { &top; print "<div align=\"center\"><center><table width=\"246\">"; print "<tr>"; print "<td align=\"left\" width=\"312\">"; print "<form action=\"http://mysite.com/cgi-bin/test.cgi\" method=\"PO +ST\">"; print "<li type=\"square\"><font size=\"2\"><i>Select files</i></font> +<BR>"; foreach $_(@thefiles) { $file_pathname = "$location" . "/" . "$_"; if (-e $file_pathname) {next;} $_ = "$FORM{'file'}"; print "&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;<input type=\"checkbox\ +" name=\"$FORM{'file'}\" value=\"1\"><b>$_</b><br>"; } print "<p><input type=\"submit\" name=\"create_list\" value=\"Create\" +></p>"; print "</form>"; print "</td></tr></table></div><table>"; print "<br><A HREF=\"javascript:history.back()\">Go Back...</A></cente +r>"; &bottom; } sub bottom { print "</table><br>"; print "<small><small>$footer</small></small>"; print "</center></html>"; exit; } sub create_list { open(FILE, ">list.txt"); foreach $_ (@thefiles) { if ($_ == 1) { print FILE "$_\n"; } } close(FILE); &finished; } sub finished { print "<center><h3>Finished...</h3>"; print "You selected $_</center>"; }

Replies are listed 'Best First'.
Re: a question about working with the forms and array
by arturo (Vicar) on Oct 09, 2003 at 16:52 UTC

    There's a lot to tackle here. First, and most important, is that you don't tell us what, precisely, isn't working. That would potentially help to narrow it down, because there are a lot of problem areas.

    • Was your opendir call in your Begin subroutine successful? If it wasn't, you wouldn't know, because it would fail silently.
      • Solution: don't let it fail silently. Rewrite that line as opendir DIR, $path or die "Can't open directory:$!\n";
    • Where are you reading the form input values from?
      • Solution : standard practice is to use CGI and import the param function to read the request parameters. You do this by putting use CGI 'param'; or use CGI qw(param); near the top of your script, and calling param('foo') to get the submitted value.
    • Where is the %FORM hash populated? If it's not populated, then all the parameter names in your HTML form will be blank. (obviously this relates to the issue above).
      • Solution : make sure it's populated. Debugging this sort of issue is easier if you run your script with use warnings; or -w in your hash-bang line. This would cause perl to warn you that you're using uninitialized values.
    The latter two are really serious problems: you have no way of reading the results of your user's interaction.

    Some larger issues that pertain to performance and readability: foreach (@thefiles) and foreach $_ (@thefiles) are functionally equivalent, but the longer version has the added bonus of not being idiomatic and also potentially confusing. You don't need to loop through an array to get its size, and you definitely don't need to sort that array and throw away the results while doing so. $count = @thefiles will get you the answer you seek there.

    Most importantly, you're not using strict in this code, which gives you the false convenience of globals; this makes it harder to understand your program, especially for someone who didn't write it. This has a further deleterious effect on your subroutines, which don't group the various functional bits of your program together in a natural way. Think about the smallest pieces of stuff your program needs to do and work from there. I would suggest that you have one subroutine that gets the list of files, sorts it, and returns it, and another to generate the checkbox form, and takes as a parameter the list of files. That should be enough to get you started.

    HTH!

    If not P, what? Q maybe?
    "Sidney Morgenbesser"

      Hello, thank you very much for your great comments and valued suggestions. I am deeply sorry for not being clear enough on my first attempt. My problem was only to learn why the values I select from my form array are not making it into the text file as they should? The last choice I select is the only value being added to the list at this time. (In the past, I had real name values written in my html form. Passing/controlling these type of values has been so simple.) The below example uses an array made directly from opening a given dir. This confuses me deeply and I have tried so very hard to find the error for weeks... I have again polished things up a bit and followed your advice. Sincerely, I don't yet fully understand how to write all the different methods using perl, (I do promise to study them for the benifit they will bring me)if I may request the example or correction to follow my current coding style. Hope I'm not being to forward, Thank you again in advance for your time and insight. Jim
      #!/usr/bin/perl -w use CGI::Carp qw(fatalsToBrowser); ############################################# $path_to_files = "/home/foo/www/dir"; $script = "$ENV{'QUERY_STRING'}"; ############################################# read(STDIN, $buffer, $ENV{'CONTENT_LENGTH'}); @pairs = split(/&/, $buffer); foreach $pair (@pairs) { ($name, $value) = split(/=/, $pair); $value =~ tr/+/ /; $value =~ s/%([a-fA-F0-9][a-fA-F0-9])/pack("C", hex($1))/eg; $value =~ s/\n/ /g;# added to strip line breaks $FORM{$name} = $value; } ############################################# print "Content-type: text/html\n\n"; $query = "$ENV{'QUERY_STRING'}"; if ($query eq "test") {&Display_Form} elsif ($FORM{'create_list'} eq "Create") {&Create_List} else {&Display_Form} #### BEGIN DISPLAY FORM #### sub Top { opendir DIR, "$path_to_files" or die "Can't open directory:$!\n"; @thefiles = sort(grep(/html$/, readdir(DIR))); closedir(DIR); $count = @thefiles; print "<html>"; print "<head><title>MY SIMPLE FILE SELECTOR</title></head>"; print "<body bgcolor =\"#FFFFFF\" text =\"#000000\" link = \"#808080\" + vlink = \"#808080\" alink = \"#808080\">"; print "<center><h2>MY SIMPLE FILE SELECTOR</h2>"; print "<br><br><small><b>The following [$count] files were found in th +e [$path_to_files] Dir.</b></small><br><br>"; print "</center>"; } sub Display_Form { &Top; print "<center><table width=\"246\">"; print "<tr>"; print "<td align=\"left\" width=\"312\">"; print "<form action=\"$script\" method=\"POST\">"; print "<li type=\"square\"><font size=\"2\"><i>Select from list</i></f +ont><BR>"; foreach (@thefiles) { $choice = $_; print "<input type=\"checkbox\" name=\"choice\" value=\"$choice\"><b>$ +choice</b><br>"; } print "<p><input type=\"submit\" name=\"create_list\" value=\"Create\" +></p>"; print "</form>"; print "</td></tr></table>"; &Bottom; } sub Bottom { print "<center><br>"; print "<small><small>$footer</small></small>"; print "</center></html>"; } #### END DISPLAY FORM #### #### BEGIN CREATION PROCESS #### sub Create_List { $choice = "$FORM{'choice'}"; @choices = split(/,/, $choice); $selected = @choices; open FILE,">list.txt" or die "Can't open file:$!\n"; foreach (@choices) { print FILE "$choice\n"; } close FILE; &Finished; } sub Finished { &Top; print "<center><h4>List created</h4>"; print "You selected ($selected):<br><li>$choice</li><br>"; print "<br><A HREF=\"javascript:history.back()\">Go Back...</A></cente +r>"; &Bottom; } #### END PROCESS ####

        Really, seriously, use CGI instead of your hand-rolled procedure. It's been battle-tested, and is amazingly convenient to use; it handles all the URL-encoding and so forth, and deals with (inter alia) URLS that read like /script.pl?foo=bar&amp;baz=bletch, which would mess yours up. CGI will cut way down on the number of lines of code in your script, and make debugging easier. In fact, it's your hand-rolled procedure that appears to be the culprit. What you have in your HTML form, as generated by the script, is something like this:

        <input type="checkbox" name="choice" value="foo.txt"> <input type="checkbox" name="choice" value="bar.txt">

        And, assuming you check both boxes, there will be two name-value pairs in the input:
        choice=foo.txt choice=bar.txt
        and when your hand-rolled procedure runs, here's what happens: it encounters the first one, and sets $FORM{'choice'} to foo.txt; when it enounters the second one, it sets $FORM{'choice'} to bar.txt. So that's why you only see the last value.

        Here's how you'd parse the form with CGI: at the top of your script, put use CGI qw(param); and to get the list of selected files, you'd simply add the one line @files = param('choice');. End of story.

        I also want to lean hard on use strict. It may seem like it will make things more painful, but it will force you to think about which subroutines need access to which data. It will make your code so much more maintainable. And, as a final suggestion, I'd suggest you look into using CGI ;)

        If not P, what? Q maybe?
        "Sidney Morgenbesser"

Re: a question about working with the forms and array
by hardburn (Abbot) on Oct 09, 2003 at 16:27 UTC

    You've given a good description of what you want your program to do, but you don't say what isn't working. Is it not putting the data into your selection box? Are you getting Internal Server Errors? Please give a better problem description.

    ----
    I wanted to explore how Perl's closures can be manipulated, and ended up creating an object system by accident.
    -- Schemer

    Note: All code is untested, unless otherwise stated

Re: a question about working with the forms and array
by diotalevi (Canon) on Oct 09, 2003 at 17:38 UTC

    Just an fyi - you should have a use strict;, a use warnings; and you can use CGI to write your HTML for you. Observe.

    #!/usr/bin/perl use strict; use warnings; use CGI ':all'; use CGI::Carp qw(fatalsToBrowser); ######################## our $PATH = "/home/foo/www/dir"; { my @files = sort grep /html$/, glob $PATH; print param() ? selection_report( @files ) : selection_list( @files ); exit; } sub selection_report { return header_template() . center( h3( 'Finished...' ) . "You selected: " . join(", ", param('files') ) ) . footer_template(); } sub header_template { return header() . start_html( -title => 'MY SIMPLE FILE SELECTOR', -bgcolor => '#ffffff', -text => '#000000', -link => '#000000', -vlink => '#000000', -alink => '#000000' ); } sub footer_template { return br() . a( { href => 'javascript:history.back()' }, 'Go Back...' ) . small( small( '...' ) ) . end_html(); } sub selection_list { my @files = @_; return header_template() . center( h2( 'MY SIMPLE FILE SELECTOR' ) . br() . br() . small( b( "The following [$#files] files were foun +d in the [$PATH] directory." ) ) ) . br() . br() . table( div( { -align => 'center' }, center( table( { -width => 246 }, Tr( td( { -align => 'left', -width => 312 }, start_form( -method => 'P +OST' ) . li( { -type => 'square' + }, font( { -size => 2 +}, i( 'Select fi +les' ) ) . br() . map( b( checkbox_ +group( 'files', + $_ ) ), @files) ) . p( submit( -name => 'cr +eate_list', -value => 'C +reate' ) ) . end_form() ) # </td> ) # </tr> ) # </table> ) # </center> ) # </div> ) # </table> . footer_template(); }
Re: a question about working with the forms and array
by freddo411 (Chaplain) on Oct 09, 2003 at 17:51 UTC
    Kudos to the other posters for their helpful advice. I concur.

    I did want to add something. Perl has an handy feature sometimes called HERE documents. They work just like quotes but even better. They start with: << followed by a string. They end with the same string at the begining of a line. (see example below.)

    Using this, you can encapsulate the entire HTML page you intend to return in one quoted chunk. Small pieces of the page can be included as perl variables that you constructed previously. Virtually none of the characters in the HERE block need to be escaped, exception for $ signs (any others?). Doing this makes your code MUCH easier to read. This technique amounts to a quick, crude Templating system -- a good thing, but that's another post. In the example below I use "MESSAGE1" as my string:

    #!/usr/bin/perl # UNTESTED use CGI; use strict; use warnings; # do some processing here # create my output strings # IN real life these would be do something real... my $myTable = "some HTML string"; my $myOtherTable= "some HTML string"; # # Print the HTML # print CGI::header; print <<MESSAGE1; <html> <body> <center> <table border=0 cellpadding=0 cellspacing=0 width=600> <tr> <td align="left"><img src="img/citi.gif" width=150></td> <td align=right><img src="img/travelers.gif" height=60></td> </tr> </table> $myTable $myOtherTable ... lots of HTML deleted from here for brevity ... </center> </body> </html> MESSAGE1

    -------------------------------------
    Nothing is too wonderful to be true
    -- Michael Faraday

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://297962]
Approved by TStanley
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others perusing the Monastery: (2)
As of 2024-04-25 20:09 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found