On Jun 16, 2004, at 12:22 PM, Naser Ali wrote:

Hello All,

Hello.

Yesterday I posted a question asking if anyone can suggest a way of
accomplishing this. In the mean while I have comeup with a quick and dirty
way of processing the data file and sorting it in an array. Once it is
sorted, then, I can do whatever however I need to. Code is attached below.
If you have any suggestions on better way of accomplishing the same task,
please share.

Your code is a good start, but there are things that could be better. I'm going to make some general comments below. Then, if you like, you can make some changes and repost and I promise to look again...


#!/usr/bin/perl

Two lines missing right here:

use strict;
use warnings;

These promise you will obey The Rules of Good Programming and Perl with pay you back with meaningful messages that help you do your job. It's a great deal.

Your code won't currently compile with these in. You'll need some changes. Mainly, you need to declare variables before you use them (or when you initialize them). Use my() for this. Example:

my $count = 0;

$want=1;
$count=0;

I seriously doubt these are needed and certainly not way up here were they tell me nothing.


open(STDIN, "test.txt");

First, if we ever ask the OS to do work for us, we need to be sure it succeeds. It could fail for many, many reasons and if it does, we want to know why.


Second, let STDIN do what STDIN is suppose to do. Use your own file handle.

Putting that together, we get:

open REPORT, 'test.txt' or die "File error:  $!";

@array=<STDIN>;

An @ called array is redundant and tells me nothing when I'm trying to read code. Use meaningful names.


my @lines = <REPORT>;

close (STDIN);

close REPORT;

print "Size of the Array is $#array\n";

Wrong. $#array is the last index, not the size.

print "The array contains ", scalar(@array), " members\n",
        "The index of the last member is $#array.\n;

print "@array\n";

for ($x=0; $x <= $#array; $x++) {

print "Index[$x] ----> $array[$x]\n";
}

Yuck. That looks like C. ;) We let are language handle those annoying details for us:


for my $i (0..$#array) {
        print "...";
}

# or, if you don't need the index...

print "$_\n" for @array;

####################################################

$k=0;

Again, meaningful names. What does this variable track, line number? Tell me that.


Do we need to track line numbers? No. They're in an array so the line they were on is index + 1. We already have that info. Looks line you are even tracking indexes, not line numbers, so we don't even need the + 1.

foreach $line (@array){

chomp $line;

if ($line =~ /Product/g) {

We don't need a global modifier on our match, we're just checking if it's in there.


if (++$count == $want) {

Are we just trying to make sure we only get the first one? See my next comment...


             print "line number is ---> $k\n";
             $FIRST=$k;

last; # end loop, we'll only get the first one

           }
        }
$k++;
}

for my $i (0..$#array) { if ($array[$i] =~ m/Product/) { $FIRST = $i; } elsif ($array[$i] =~ m/System Totals/) { $LAST = $i; } }

$i=0;
$want=1;
$count=0;

I doubt we need any of these. See comments above.

foreach $line (@array){

chomp $line;

        if ($line =~ /System Totals/g)      {
           if (++$count == $want) {
             print "line number is ---> $i\n";
             $LAST=$i;
           }
        }
$i++;
}

Just like the previous loop.

$j=0;

for ($ii=$FIRST; $ii <= $LAST; $ii++) {

    $array2[$j] = $array[$ii];
    $j++;
}

What are we doing here? Making another array of first to last?

my @first_to_last = @array[$FIRST..$LAST];

for ($y=0; $y <= $#array2; $y++) {

print "Index[$y] ----> $array2[$y]\n";
}

print "FIRST--> $FIRST and LAST--->$LAST\n";

Let me comment on one last issue. You're "slurping" the file or reading the whole thing into memory in one move. Then you have to do a dance, to get what you want. Too much hassle.


The right way is to only read in what you want and deal with that. You handle this line by line as it's coming in. See if you can setup something like that.

Good luck and yell if you need help...

James


-- 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