Well, we'll assume you've got strict and warnings turned on, and that you've checked the return values when you opened your LOG and LOGFILE filehandles :-)
It looks pretty good stylistically speaking. A couple of points though: Log files can be large --- is there any other reason you need to read it into an array? Also, if your log file is empty then $start_time will be undef rather than 0 --- in fact, in your algorithm, $start_time can never be 0 (it will be undef if the file was empty or the $date was never found, and it will be greater than 0 if the $date is found ... even if is found in the first line of the array (position 0) ... oops).
If you do not need @input except for in this snippet, then you do not really need this array at all, and you won't need the $counter, $start_time, or $end variables either. You can simply read through the file and print everything between the first match of $date and the end of file (eof) --- your entire snippet may be reduced to:
while(<LOG>){ next unless /$date/ .. eof(LOG); print LOGFILE; }
It isn't only shorter --- it helps avoid logic mistakes like for instance: Consider what happens in your code if the $date is found in the first line of the file (element 0 in the array). Your code has already incremented the counter, so your output would skip the first line of the file (the 0'th element of the array that actually contains the search term). Trying to fix that by incrementing the counter at the bottom of the loop instead would set your start_time properly to 0, but then your "empty file" test would come into play (start_time is 0, but the file wasn't empty). Lastly, if the date was never found, what is going to happen in your code (with or without changing the location of the $counter increment)?
When you find yourself creating additional variables that have little or nothing to do with problem at hand, and more to do with the details of your solution (book-keeping variables most commonly), you should ask yourself if this is best way to express your solution (such variables are often referred to as 'synthetic variables', see references at end of this post). In the reduced snippet, all of the identifiers relate to the problem space: $date is the search term, LOG is the input file, eof(LOG) refers to the end of the input file, and LOGFILE is the output file. The entire logic of the snippet is captured in the range operator (see perlop).
If you did require the @input array elsewhere in your code, you can still reduce your logic in a similar manner:
my @input = <LOG>; foreach (@input) { next unless /$date/ .. undef; print LOGFILE; }
Using the range operator like this may seem like a bit of a trick (or at least esoteric enough that not everyone would think of it) --- so let's consider an algorithm that doesn't use it at all but you still want the array. We could begin as you did, trying to find the first valid location in the array (but we have the problem that the first valid location might be 0, and how will we tell if the location is 0 because that's where we found the search term, or its 0 because we never found the search term --- we could use defined, but now we not only have a synthetic variable, we have additional synthetic code to determine what some of its values mean). What we want in this case is a simple flag to tell us if we've found the search term or not --- yes it is a synthetic variable, but it is the minimum required if we aren't using the range op, it only means one thing (did we find it, true or false), and we can restrict its scope:
my @input = <LOG>; { my $found = 0; foreach (@input) { $found = 1 if /foo/; print LOGFILE if $found; } }
Dominus has a couple of articles the mention synthetic variables
at perl.com that are well worth reading:
http://www.perl.com/pub/a/2000/11/repair3x.html
http://www.perl.com/pub/a/2000/06/commify.html
In reply to Re: Monk Specs.?
by danger
in thread Monk Specs.?
by draper7
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |