in reply to Re: My vacation depends on it
in thread My vacation depends on it

OK here's my code so far....
#!/usr/bin/perl -w print "Content-type: text/html\n\n"; #Here I am getting the values from #the GET form. foreach $pair (split('&', $form)) { if ($pair =~ /(.*)=(.*)/) { # found key=value; ($key,$value) = ($1,$2); # get key, value. $value =~ s/\+/ /g; # substitute spaces for + signs. $value =~ s/%(..)/pack('c',hex($1))/eg; $inputs{$key} = $value; # Create Associative Array. } } #This next assigns assigns the values to variables $nombre = $inputs{'nombre'}; $encabezado = $inputs{'encabezado'}; $cat = $inputs{'cat'}; #Now I created this rudimentary statements that by seeing #which categ +ory I chose, select which file I am going to #move the link to. if($cat == t){ $catfile = "notitec.html"; } if($cat == a){ $catfile = "notiagro.html"; } if($cat == d){ $catfile = "notideportes.html"; } if($cat == h){ $catfile = "notihogar.html"; } if($cat == r){ $catfile = "notirecrea.html"; } if($cat == s){ $catfile = "notisociales.html"; } #Now I open the current headline and read it to an array #but changing + locally the separator character so I don't #get a straight line of h +tml code { $current = "/var/www/html/notidia.html"; open(DIA, "< $current") or die("Couldn't open $current\n"); local $/ = >; @file_data = <DIA>; close DIA; } #now open the category file and insert the previous data $catnoticia = "/var/www/html/$catfile"; open(NOT, "> $catnews") or die("Couldn't open $catnews\n"); foreach $line (@file_data) { print "$line"; } close NOT; #that's what I got so far. I think I got the idea right but #didn't ex +press it correctly on the code (duh) #If I have some variable names wrong its because I changed #the names +to english or something understandable to you so #you can get a bette +r idea.

Replies are listed 'Best First'.
Re: Re: Re: My vacation depends on it
by arturo (Vicar) on Apr 03, 2001 at 23:30 UTC

    OK, here are some general pointers:

    1. use CGI qw(:param) to process your form data; handrolled stuff is messy, hard to maintain, and insecure. See use CGI or die; for more about why.
    2. use strict always and run under -w, the latter especially when you're developing. Perl will catch all sorts of errors you won't. As things stand, your script could fail for lots of reasons in lots of places and you'd never know why.

    our own Ovid has written a CGI tutorial guide, check his homenode for the link (follow the link in this paragraph)

    Now, some specifics:

    What your ifs are doing is done more maintainably with a hash. I like to do this at the top of complex scripts:

    my %actions = ( option1 => \&process_option1, option2=>\&processoption2 ); my $option = param('option'); $action{$option}->() or default_action(); exit;

    And so forth. That nasty-looking \& thing creates references to subroutines, which are called by the $action{$option} line. Here, what you should do is create a hash whose keys are as you have them, and whose values are the files that each kind of action works on:

    my %file_hash = ( a=>'notiagro.html', d=>'notideportes.html' # and so on ); my $catfile = $file_hash{$cat} || 'default value';

    I know that doesn't get you everything you need. You have a start on the harder parts, but get this part working first and then come back to those.

    For finding the link to extract and replace, you need to think about how your file looks. I can't help you with that with the information I have now, but I've gone on long enough so far. Hope this gets you over a hurdle or two.

    Philosophy can be made out of anything. Or less -- Jerry A. Fodor

      As much as I like a(some) nice %hash, I also like a good switch for clarification, it beats those if'n'else's Check out perlman:perlsyn and How do I create a switch or case statement? for more clarification.
      $cat = 'ata'; SWITCH: for ($cat) # make $_ = $cat, you could do it by hand { # syntaxt # whatever test on $_ && do { $this = 'if test passed'; last;}; (length $_ > 1) && do { die "\$cat must be one letter not: `$cat'" +; last;}; /t/ && do { $catfile = "notitec.html"; last; }; /a/ && do { $catfile = "notiagro.html"; last; }; /d/ && do { $catfile = "notideportes.html"; last; }; /h/ && do { $catfile = "notihogar.html"; last; }; /r/ && do { $catfile = "notirecrea.html"; last; }; /s/ && do { $catfile = "notisociales.html"; last; }; die "unknown value for \$cat: `$cat'"; # could just as easily be the default value }
      P.S. Are you a sadist?(no -w strict, you must -w strict)

      update: I agree tilly, not that i'd dare disagree with you ;). I feel <10 and not gonna expand, is a good time to use one.

       
      ___crazyinsomniac_______________________________________
      Disclaimer: Don't blame. It came from inside the void

      perl -e "$q=$_;map({chr unpack qq;H*;,$_}split(q;;,q*H*));print;$q/$q;"

        The scanning logic of a switch is flexible and easy for you to understand and see, but if you have long lists of options that you are processing within a loop, it will be a lot slower than dispatching on a hash lookup. If you can arrange to do it the latter way, it is generally worth doing so.

        Just something to be aware of.

Re: Re: Re: My vacation depends on it
by dws (Chancellor) on Apr 04, 2001 at 00:06 UTC
    Everything arturo says, plus:

    1. The string comparison operator is "eq", not "==".

    2. You're opening /var/www/html/$catfile for writing, but you're not actually writing to it.

    Replace   print "$line"; with   print NOT $line;