On Dec 12, 2003, at 6:20 PM, Kenton Brede wrote:

I've cobbled some code together that will allow me to parse a file
snarfing 4 lines which consist of server name and Daily output of
bandwith usage.  I've pasted an example of what I have at the bottom of
this mail.  If anyone would like to take the time to show me how it
should really be done I would apreciate it.  I'm not exactly a
programming wonder and trying to learn.

First, just let me say that you've done very well. You use strict and warnings, your code works and you want to know how to make it better. I can't ask for much more than that! :)


#!/usr/bin/perl
use warnings;
use strict;

my @lines;

while ( <DATA> ) {
    push ( @lines, $_ );
}

First, let's simplify:


my @lines = <DATA>;

That's the same thing you have above, without needing a loop. The read of DATA happens in list context here, because we're assigning to an array. In list context, the reads returns all the lines in a list. We assign that and we're done.

for ( my $count = 0; $count <= $#lines; $count++ ) {

That loop is pretty C-ish. :D In Perl, we usually write that:


for my $count (0..$#lines) {

if ( $lines[$count] =~ m/\*/g ) { # grab server names

Okay, we don't need a global modifier on that regex, since we're just looking for one *. If we're looking for a * at the beginning of the line, we should say that with /^\*/ instead.


print "$lines[$count]";

print $lines[$count];


Don't quote a variable unless you're interpolating it into some larger string. If you need a string to print, Perl will handle that.

    }
    if ( $lines[$count] =~ m/Daily/g ) { # grab daily output

Again, no /g needed.


print "$lines[$count] $lines[$count+1] $lines[$count+2]\n";

Quotes ARE needed here. ;)


    }
}

Now, the problem with the approach you're using above is that you're slurping the whole file into memory. That's nothing with the data set you show here, but it could be a problem with huge files. It's always better to process as few lines at a time as you need. Handle them and move on. Keep your footprint small.


Here's a rewrite that processes as the lines come in, to give you some ideas:

while (<DATA>) {                  # read until we find...
        if (/^\*/) { print; }                   # the name, which we print, ...
        elsif (/\bDaily\b/) {           # or the daily totals...
                print;                          # which we print...
                while (<DATA>) {  # and then print everything until the next blank line
                        print;
                        last if /^\s*$/;        # exit inner loop on a blank line
                }
        }
}

Hope that helps.

James

__DATA__

* Leo

# Bandwidth Usage
eth0 Total for 106.95 days:
    RX 307.28 MB
    TX 768.05 MB
eth0 Daily average:
    RX 2.87 MB
    TX 7.18 MB

* Buffy2

# Bandwidth Usage
eth0 Total for 14.70 days:
    RX 141.28 MB
    TX 2.03 MB
eth0 Daily average:
    RX 9.61 MB
    TX 0.14 MB

--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>




--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>




Reply via email to