Better rewrite of multiple ifs

[Nik]

Hello Monks, I was trying to change this snippet of code:

   if( !$name )    { print font( {-size=>4, -color=>'Lime'},  'Ξέχασες
+ να μας πείς ποιός είσαι!<p>' );       $i=1; }
   if( !$pray )    { print font( {-size=>4, -color=>'Lime'},  'Δεν σχο
+λίασες την ευχή!<p>' );                $i=1; }
   if( !$remark )  { print font( {-size=>4, -color=>'Lime'},  'Δεν θα 
+μας πείς για την εμπειρία σου?<p>' );  $i=1; }
   if( !$email )   { print font( {-size=>4, -color=>'Lime'}, 'Συμπλήρω
+σε το email σου!<p>' );               $i=1; }
   exit 0 if ($i!=0);

to a better maintenable one since something got me these days to perfect all my cgi scripts :-)

   my %error_msgs = (
       name   => "Ξέχασες να μας πείς ποιός είσαι!",
       pray   => "Δεν σχολίασες την ευχή!",
       remark => "Δεν θα μας πείς για την εμπειρία σου?'",
       email  => "Συμπλήρωσε το email σου!"
   );

   my $error_found;

   for ( keys %error_msgs) )
   {
       unless ( $error_msgs){$_} )
       {
           print div( {class=>'tip'},  $%error_msgs{$_} );
           $error_found++;
       }
    }

   exit 0 if $error_found;

Unfortunately it aint working as expected.

ps. ΧΡΗΣΤΟΣ ΑΝΕΣΤΗ to all the world!

[Dietz]

You've got 3 characters too many:
for ( keys %error_msgs) )
for ( keys %error_msgs) - removed trailing round bracket

unless ( $error_msgs){$_} )
unless ( $error_msgs{$_} ) - removed right round bracket after $error_msgs

print div( {class=>'tip'},  $%error_msgs{$_} );
print div( {class=>'tip'},  $error_msgs{$_} ); - removed percent sign

Update: Formatting

[Nik]

Thank you!
I though it was something in the logic but i guess i was too tired to see that extra brackets.

[eibwen]

Don't forget to use strict; and use warnings; (or -w). Additionally, you may be interested in use diagnostics; which expounds upon perl's error messages, which returned (for the above code):

Bareword found where operator expected at ./tmp line 20, near "$%error_msgs" (#1)
    (S syntax) The Perl lexer knows whether to expect a term or an operator.
    If it sees what it knows to be a term when it was expecting to see an
    operator, it gives you this warning.  Usually it indicates that an
    operator or delimiter was omitted, such as a semicolon.
Read more... (2 kB)

Additionally, remember that $name != $error_msgs{'name'}; I think you may be looking for something like:

my %errs = (
    name => 'Unspecified',
    email => 'Invalid'
);

my %fields = (
    name => 'Bob',
    email => undef
);

foreach (keys %fields) {
    print $errs{$_} unless $fields{$_};
}

Given the above implementation (in the OP code), you may also be interested in warn and die.

[Courage]

interestingly to note, capital greek letters in your PostScriptum look okay, whereas none of ordinary greek text looks good. Each letter is replaced by some accented character.

See 33Kb screenshot at http://vkonovalov.ru/images/snapshot1.png

Soomething with importance of your PS note! :):):)
addition Didn't you entered greek letters at bottom as something like Σ ?
May be it worth re-encoding entire your script in such a way, in one go?

[tlm]

The only way your scheme will work is by turning $name, $pray, etc. into global variables and using the keys of %error_msgs as symbolic references to these globals; i.e.:

   for my $var ( keys %error_msgs ) {
       no strict 'refs';
       unless ( $$var ) {
           print div( { class=>'tip'},  $error_msgs{ $_ } );
           $error_found++;
       }
    }

(By the way, get in the habit of running perl -c my_program.pl on your code, just to make sure your code at least compiles.)

It is almost certainly not a good idea for you to be doing this (see this series of articles by the Dominus for why you'd want to stay away from symbolic references; and also this). Another (less serious) downside to your scheme is that you lose control over the order in which errors are checked and reported, since it is dictated by the the order in which keys iterates over %error_msgs's hash keys.

A better alternative would be to use parallel hashes (untested):

my @fields = qw( name pray remark email );

my %error_msgs;
@error_msgs{ @fields } =
  ( "Ξέχασες να μας πείς ποιός είσαι!",
    "Δεν σχολίασες την ευχή!",
    "Δεν θα μας πείς για την εμπειρία σου?'",
    "Συμπλήρωσε το email σου!" );

# ...Just one thing, Dude.  D'ya have to use s'many cuss words?

my %info;

# initialize %info

for my $field ( @fields ) {
  unless ( defined $info{ $field } ) {
    print div( { class => 'tip' },  $error_msgs{ $field } );
    $error_found++;
  }
}

[bradcathey]

I asked a similar question a while ago and got some good answers.