http://qs1969.pair.com?node_id=324046


in reply to The third 'if'

Some helpful indentation would point out troubles with your code.

Nested magical while loops will walk on each other's $_. I don't think that is hurting you here, but it will someday.

You have a logic bug, your code says, 

while ( lines_to_read() ) {
    if ( the_line_is_the_right_header()) {
        if ( the_line_also_has_impressions) {
            # impression things
        }
    }
}

[download]
No line satisfies both conditions. You need to last out of the inner loop if the header is wrong. If the header is right keep saying next until you find "Impressions:" or run out of lines.

Your impression handling code treats a string with leading spaces (from split /:/) as a number. That will always evaluate to zero. [ysth++ is correct. I still think:] split /:\s*/ will work better.

Update: Here is the whole loop in pseudocode with last and next applied to fix the logic, 

while (files_to_read()) {
     next unless the_line_is_the_right_header();
     while ( lines_to_read() ) {
         next unless the_line_has_impressions();
         # impression things
         last;    # no need to look further in this file
     }
}

[download]

After Compline,
Zaxo

Replies are listed 'Best First'.
Re: Re: The third 'if'
by ysth (Canon) on Jan 26, 2004 at 02:12 UTC
    Leading and trailing spaces are silently ignored in string->numeric conversion (except for the special "0 but true"). So "  3   " == 3, with no warnings.
[reply]
[d/l]
Re: Re: The third 'if'
by danield (Novice) on Jan 26, 2004 at 17:29 UTC
    Hello Zaxo, Unfortunatelly I don't know how to implement 'last' or 'next', could you please be more specific?
    In meantime I have tested a suggestion of a different person so the code now looks like: 
    #!usr/bin/perl -w

#use strict;
use Fcntl qw[:flock];

my $impressions = 0;
my $iofile = '/other/scripts/daniel/input/c07_impressions_io.info';

open (IO, $iofile) || die("Could not open file 1!");

while (<IO>) {
      chop;
      my ($FH, $output, $file2check, $month, $year) = split (/\s+/, $_
+);

open OUT, ">> $output";

chdir $FH or die "$!";

while (glob $file2check) {
    open FH, $_ or die $!;
    flock FH, LOCK_SH or die $!;
    while (<FH>) {
        chomp;
    if ( / Summary Log \(generated:/ ) {
        print "Found Sumary Log, checking the month and year.\n";
        my($emptyspce, $summary, $log, $generated, $day_word, $monthfi
+le, $day_number, $time, $timezone, $yearfile) = split(/\s+/, $_);
        print "The current file has $monthfile and $yearfile\n";
        print "And I am looging for $month and $year\n";
    if (    $monthfile eq $month
        and $yearfile  eq $year){
        print "File has $yearfile and $monthfile in it. Now Looking fo
+r Impressions.\n";
        my $found = 0;
        until ($found) {
        my $line = <FH>;
          if ($line =~ /Impressions:/ ) {
          $found++;
          print "Found Impressions, now splitting.\n";
          my($text, $value) = split(/:\s*/, $_);
          print  $impressions += $value if ($value =~ /\d+/);
          print "\n";
        }
      }
    }
      }
    }
    close FH or die $!;
  }
}
print OUT 'Total impressions: ', $impressions or die $!;

    [download]

    And the output I am getting now is: 
    Found Impressions, now splitting.
Argument "Sat Nov  1 00" isn't numeric in add at c07c_imp.pl line 39, 
+<FH> chunk 13.
0
Found Sumary Log, checking the month and year.
The current file has Nov and 2003)
And I am looging for Nov and 2003)
File has 2003) and Nov in it. Now Looking for Impressions.
Found Impressions, now splitting.
Argument "Sat Nov  1 00" isn't numeric in add at c07c_imp.pl line 39, 
+<FH> chunk 13.
0
Found Sumary Log, checking the month and year.
The current file has Nov and 2003)
And I am looging for Nov and 2003)
File has 2003) and Nov in it. Now Looking for Impressions.
Found Impressions, now splitting.
Argument "Sat Nov  1 00" isn't numeric in add at c07c_imp.pl line 39, 
+<FH> chunk 13.

    [download]
[reply]
[d/l]
[select]
      I think your line 38 should be my($text, $value) = split(/:\s*/, $line); instead of my($text, $value) = split(/:\s*/, $_);. It goes back to what Zaxo said about $_ haunting you. :)

      Your match in line 39 will be true if there is any digit in $value, if you want for only digits try /^\d+$/ instead, but in your case (where you know $value will have a number in it) you don't actually need that if statement.

[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom