about perl condition statement go wrong question

Question

We use Perl scripts to check if condition.

##code(t1) is as belows:

my @results = (93, 4, 0);

my @param_array = (
    [ "50", "<", "stat1", ],
    [ "1", "<", "stat2", ],
    [ "3", "<", "stat3", ],
);

for ($i=0; $i < @results; $i++) {

  print (" " . $results[$i] . " " . $param_array[$i][1] . " " . $param_array[$i][0] . " ");

  if  ( $results[$i] + 0 < $param_array[i][0] + 0 ) {

    print "  beend";

  }
  else {
    print "  end111";

  }
}

The result is strange. When 95<50, if condition is not true, and it prints end111. I think the statement is right.

But when 4 < 1, if condition is not true, it also prints beend. I think the statement is wrong.

Why does this happen?

###result is as below
perl t1
   93 < 50   end111   
   4 < 1   beend  
   0 < 3   beend 

Answer 1

Here's your original program:

my @results = (93, 4, 0);

my @param_array = (
    [ "50", "<", "stat1", ],
    [ "1", "<", "stat2", ],
    [ "3", "<", "stat3", ],
);

for ($i=0; $i < @results; $i++) {
  print (" " . $results[$i] . " " . $param_array[$i][1] . " " . $param_array[$i][0] . " ");
  if  ( $results[$i] + 0 < $param_array[i][0] + 0 ) {
    print "  beend";
  }
  else {
    print "  end111";
  }
}

As toolic already noted, your use of i in $param_array[i][0] is the problem, and you would have found that with strict and warnings. Since Perl converts values in array indices to integers, undef values convert to 0, and you end up getting the same part of the array (element 0) each time.

However, there are a couple of other things that you can do to avoid these sorts of problems.

You have a debugging statement, but it's misleading you because it's using different values than the actual code (a mistake that is not uncommon for me). You think you are using one set of values but actually using another. To get around this, you might collect the values into meaningfully named variables first, then always use those names to refer to the values. This way everything refers to the same thing:

for( ... ) {
    my $left       = $results[$i];
    my $operation  = $param_array[$i][1];
    my $right      = $param_array[$i][0];

    print " $left $operation $right   ";
    say $left < $right ? 'beend' : 'end111';
    }

But that looks very close to a subroutine, where the argument list serves as the central source of truth. I generally don't like to output from subroutines, so I return a string that I can output from the for loop. That's not a big deal though:

for( ... ) {
    say 
        compare_things( 
            $results[$i], 
            $param_array[$i][1], 
            $param_array[$i][0] 
            );
    }

sub compare_things {
    my( $left, $operation, $right ) = @_;

    my $string = " $left $operation $right   ";
    $string .= $left < $right ? 'beend' : 'end111';
    }

Or, with subroutine signatures (still an experimental feature, but that I use everywhere):

use v5.20;
use feature qw(signatures);
no warnings qw(experimental::signatures);

sub compare_things ( $left, $operation, $right ) {    
    my $string = " $left $operation $right   ";
    $string .= $left < $right ? 'beend' : 'end111';
    }

This separates the concerns a bit. The for loop's job is merely to select the appropriate arguments and give them to something else. Since I've given that something else a name (compare_things), that complexity disappears from the loop and allows me to more easily recognize when the loop isn't doing its one job. It goes the other way too: the subroutine is not responsible for picking the values, so it can concentrate on the values that something else has already selected.

Although we've all made some pretty long loop blocks, when I get to about half a screen size (for whatever that is for you), I start to think about moving stuff out to named routines (and let's not consider hot loops at the moment because you already know what's going on there if you have that objection :)

There's another trick that's similar. When you have a multilevel data structure, drill down immediately so you don't have to type out the whole thing every time. This fixes all the keys or indices down to the last level. It looks a bit verbose here, but in more complicated slicing situations, it saves a lot of line-noise and again ensures everything refers to the same thing:

for( ... ) {
    my $left       = $results[$i];
    my $params_ref = $param_array[$i];
    my $operation  = $params_ref->[1];
    my $right      = $params_ref->[0];

    print " $left $operation $right   ";
    print $left < $right ? 'beend' : 'end111';
    print "\n";
    }

In bigger programs, those 0 and 1 indices can become a problem, especially if you change the number of elements or order of data. They are sometimes called "magic constants". You can give those labels too so everything in your application uses the same values to mean the same thing. The constant pragma comes with Perl, but there are other ways to get the same effect:

use constant VALUE => 0;
use constant OPERATION => 1;

for( ... ) {
    my $left       = $results[$i];
    my $params_ref = $param_array[$i];
    my $operation  = $params_ref->[OPERATION];
    my $right      = $params_ref->[VALUE];

    print " $left $operation $right   ";
    say $left < $right ? 'beend' : 'end111';
    }

Modules such as POSIX, Socket, and Unix::Sysexit, among many other modules, export constants for the same reason (as well as the fact that they may be different actual values on different platforms).

You've used the traditional C-style for loop, and that works, but it also makes you do the tradition C extra work. Starting with v5.12, each works on arrays too. You can get the index and value at the same time. This doesn't have loop math:

use v5.12;
while( my( $i, $left ) = each @results ) {
    my $operation  = $param_array[$i][1];
    my $right      = $param_array[$i][0];

    print " $left $operation $right   ";
    say $left < $right ? 'beend' : 'end111';
    }

There's another thing that I often like to do, and it betrays my background in math and physics. I like to line up things that are the same. If they are the same, they should line up, and if they don't line up, I've done something wrong. Instead of this (where I've already lined up the =):

    my $left       = $results[$i];
    my $operation  = $param_array[$i][1];
    my $right      = $param_array[$i][0];

I might be inclined to push over that first line so the same index is also aligned:

    my $left       =     $results[$i];
    my $operation  = $param_array[$i][1];
    my $right      = $param_array[$i][0];

This sort of visual clue shows me how things are connected and are alike by highlighting their sameness. When we see structure like that, we often see less complexity because the parallel structure makes several disconnected things into a single pattern that's quick to understand. I often catch bugs this way because I see that in some line I've done something slightly different, such as misordering arguments, leaving off an argument, or whatever.

But, that's trepasses into style issues and the people around you might have different ideas. Other people understand things differently. Salt for taste.

Answer 2

You should add use warnings; to your code, and you should see warning messages like:

Unquoted string "i" may clash with future reserved word
Argument "i" isn't numeric in array element

You should also add use strict; to your code, and you should see a compile error message like:

Bareword "i" not allowed while "strict subs" in use 

You need to change i to $i. Change:

if ( $results[$i] + 0 < $param_array[i][0] + 0 ) {

to:

if ( $results[$i] + 0 < $param_array[$i][0] + 0 ) {

This produces the following output, which I assume is what you want (although you didn't explicitly show your expected output):

 93 < 50   end111
 4 < 1   beend
 0 < 3   beend

---

Note: you also need to declare $i before your for loop to satisfy strict, if you already haven't done so:

my $i;

Here is a tidy version of your code with the fixes in place (using perltidy):

use warnings;
use strict;

my @results = ( 93, 4, 0 );

my @param_array = (
           [ "50", "<", "stat1", ],
           [ "1", "<", "stat2", ],
           [ "3", "<", "stat3", ],
);

for (my $i = 0 ; $i < @results ; $i++ ) {
    print( " " . $results[$i] . " " . $param_array[$i][1] . " " . $param_array[$i][0] . " " );

    if ( $results[$i] + 0 < $param_array[$i][0] + 0 ) {
        print "  beend";
    }
    else {
        print "  end111";
    }
    print "\n";
}