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

Hello... I am having some trouble with a foreach loop... I think my if-logic is flawed... I was wondering if anyone mite see my mistake..? I am processing date selected files from a directory... Trying to process the logfile from today and yesterday... ie. files with a filestamp of 20030312 & 20030313... My if-logic failure seems to be processing the log file more than once. Do I need to exit from the foreach loop before I close the if {} ..?

I can provide more input to this question if needed...


Thank you... Darrick...


@contents contains filenames to be processed
$pth path to the directory
$logfile is the assembled path/file name
&filestamp is a sub to retrieve ->mtime (File::stat)
$filestamp is retrieved filestamp ie. '20030313'
$datestamp is current day stamp ie. '20030313'


foreach $content (@contents) {
 #print "$content\n";
 $logfile = "$pth" . '\\' . "$content";
 #print "$logfile ";
 #print "$datestamp\n";
 foreach ($logfile) {
  &filestamp ('$logfile');
   if ($filestamp == $datestamp) {
    print "$logfile --> ";
    print "Is current, processing\n";
    open(FILE, "$logfile"); # open logfile for reading
    &do_file_processing_sub;
    close(FILE);
  }
  elsif ($filestamp == ($datestamp -1)) {
    print "$logfile --> ";
    print "Is one day old, processing\n";
    open(FILE, "$logfile"); # open logfile for reading
    &do_file_processing_sub;
    close(FILE);
  }
   else {print "$logfile is out of date range\n";}
  }
 }
}

Replies are listed 'Best First'.
Re: If logic fails foreach mistake
by dga (Hermit) on Mar 13, 2003 at 20:41 UTC

    By your code, I assume that $filestamp gets set as a side effect of the filestamp function? This would be more easily followed if you returned the timestamp from the function and called it somewhat like.

    my $filestamp=filestamp($logfile);

    One problem I notice right away is that you are using single quotes to your argument to filestamp which means it always tests the string $logfile rather than the variable contents. So its likely that $filestamp is always set to 0 or more likely undef.

    Unless you are using perl 4 then the & is optional for functions which have the ()'s after them. (and optional in other cases also)

    Update:

    Also the second foreach doesn't appear to be needed since each pass through the outer foreach will only return 1 logfile name to be processed.

[reply]
[d/l]
Re: If logic fails foreach mistake
by jasonk (Parson) on Mar 13, 2003 at 20:43 UTC

    You don't indicate what the filestamp() subroutine is doing, but it if it doesn't contain an eval, then the datestamps do not contain what you think they contain. By putting single quotes around $logfile when you pass it to filestamp(), you have prevented it from being interpolated, so instead of a logfile name, you are actually just passing that subroutine the word 'logfile' preceded by a dollar sign.

    Also, you have a fundamental flaw in your calculation that determines if the log is from yesterday. If today is the first day of the month, then subtracting one from the date string won't get you yesterday, use something like Date::Calc instead.

    And that inner 'foreach($logfile) {', while not causing problems for you, doesn't actually do anything.

    We're not surrounded, we're in a target-rich environment!
[reply]

      Come to think of it maybe replace the entire filestamp deal with  if( -M $logfile < 1 ) #less than 24 hours ago or the like

[reply]
[d/l]
Re: If logic fails foreach mistake
by Ineffectual (Scribe) on Mar 14, 2003 at 00:57 UTC 
    use IO::File;

foreach my $content (@contents) {
  $logfile = $pth .'\\' . $content;
  my $fs = filestamp($logfile);
  if ($fs == $datestamp) {
    print "$logfile --> is current, processing\n";
    my $fh = new IO::File "$logfile" || die "Error $logfile: $!\n";
    processFile($fh);
    $fh->close;
  } elsif ($fs == ($datestamp -1)){
    print "$logfile --> is one day old, processing\n";
    my $fh = new IO::File "$logfile" || die "Error $logfile: $!\n";
    processFile($fh);
    $fh->close;
  } else {
    print "$logfile is out of date range\n";
  }
}

    [download]
    You don't need multiple foreach loops and if you're going to pass a filehandle to a sub, you should probably use IO::File to do so.

    HTH
    'Fect
[reply]
[d/l]
Re: If logic fails foreach mistake
by dbrock (Sexton) on Mar 14, 2003 at 16:23 UTC

    Hello all... Thank you very much... I found my mistake (I'm new to perl still on the learning curve)... The code listed below is what i am using... I would like to thank you Monks for taking the time to assist me... I have discovered that I enjoy using PERL... I have so much more that I want to discover with it...

    Thank's Darrick...



    sub get_logfiles {
     opendir (Directory, $pth) or die "Cannot Open Directory";
     @contemps = readdir(Directory);
     closedir(Directory);
     splice(@contemps, 0, 2);
     foreach (@contemps) {
      if (m/^[B|b].+.txt$/) {
       push(@contents,$_)
      }
     }
     undef(@contemps);
     sort(@contents);
     foreach $content (@contents) {
      $logfile = "$pth" . "\\" . "$content";
      if( -M $logfile < 1 ) { #less than 24 hours ago
      #if( -M $logfile < 13 ) { #less than 13 days ago
       open(FILE, "$logfile"); # open logfile for reading
       print "   ***$logfile is in specified date range - processing***\n";
       &do_log;
       &write_out;
       close(FILE);
      }
      else {print "   ***$logfile is out of date range - skipping***\n";
      }
     }
    }
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom