Foreach Complications

[ImpalaSS]

Hello,
I have yet another problem. I am in the process of writing a script. What this script does for the engineers here, is when they select a site (cgi) and click submit, the script gathers a table of information for that site, along with all of its neighbors. How i did this was, i used a preexisting script that i wrote, which actually parsed the data, and made it a subroutine. I then made an array, which had every "netid" for the neighbor sites. The subroutine, takes the "net-id" and selects the data, but it also has 2 more variables, which is where my problem arrises. See the other 2 variables are simply used to title the column produced by the subroutine.
So this is how i did it. I have 3 arrays, one with the netids (crucial for the data) one with the siteid(used for title) and one with the sector number (again, used for title). The for each loop is:

$i=0;
$index=@netids;
print "<TABLE>";

foreach $id(@netids){
    print"<td>";
    print "<pre>";
    PRINT_INFO ($id,$nums[$i],$sects[$i]);
    print "</pre>";
        $i++;    
}

I needed the $i in there, as a counter to ensure that the second two variables getting sent to the subroutine are the correct ones. When i actually print out the lines that are sent to the subroutine using

print "PRINT_INFO ($id,$nums[$i],$sects[$i])";

i get this..

PRINT_INFO (2515,0040,1)
PRINT_INFO (9479,0040,2)
PRINT_INFO (2529,5228,1)
PRINT_INFO (9198,5228,2)
PRINT_INFO (9399,5458,1)
PRINT_INFO (12344,0473,3)
PRINT_INFO (14119,5335,1)
PRINT_INFO (14120,5335,2)
PRINT_INFO (14248,0267,3)
PRINT_INFO (19341,0314,1)
PRINT_INFO (19343,0314,3)
PRINT_INFO (19332,5268,1)
PRINT_INFO (19333,5268,2)

So i know the data being sent to the subroutine is complete and correct. This is how the sub routine parses the data

my ($netid, $sitename, $sector) = @_;

Now, from the actual data that the subrourine prints, the data itself is correct. However, the titles are not, it prints the same title a few times, or goes out of order, all kinds of wierd stuff, but the actual data, is correct.
Thanks In Advance

Dipul

[tilly]

Sounds like it is time to refactor.

First of all I would strongly recommend cutting down on the globals. For an explanation of why see RE (tilly) 3: redeclaring variables with 'my'. After that, well I don't like having to keep multiple arrays in sync. It is too easy to get it wrong. Have you considered using anonymous hashes? An array of anonymous hashes with information about the title, sector, and the data is harder to make mistakes with. If you want you can even make them into objects and hide them off in some class.

If you try you can probably remove all need to ever think about what position in a particular data structure your data is. Push that accounting down to Perl and your program will be much more reliable, easier to write, and if that wasn't enough it will be easier to read as well. :-)

[$code or die]

Gotta ++ tilly. I remember having some problems with anonymous hashes when I first started using them, but now I can't imagine not using them. Its much easier to store complex data. I use them all the time now - especially when I am handling several database records at a time.

I recommend looking at Section 1.3 of Advanced Perl Programming. It's on Nested Data Structures. Chapter 1 is a good starting point for info on references too. I just found this some basic info about Nested Data Structures at This Site

I also recommend using Data::Dumper: It allows you to dump your variables and it nicely formats nested Data Structures.

If you buy Advanced Perl Programming use the above link as PerlMonks gets some commission.

[kilinrax]

If the right information is being sent to PRINT_INFO, then the problem probably resides within that subroutine, which you haven't included in your post (hint, hint).

I would also humbly suggest cleaning up the html your script produces (which may be the problem if you're looking at it's output through a browser), and also that you probably want to be using 'for($i=0; $i<=$#netids; $i++) { ... }' rather than 'foreach $id(@netids){ .. }' as you are iterating over three lists rather than just one:

my $i;
print "<table>\n";

for ($i=0; $i<=$#netids; $i++){
    print " <tr>\n  <td>\n   <pre>\n";
    PRINT_INFO ($netids[$i],$nums[$i],$sects[$i]);
    print " </tr>\n  </td>\n   </pre>\n";
}

print "</table>\n";

[ImpalaSS]

The subroutine is rather lengthy, so let me just post the relevant parts:


sub PRINT_INFO {
  my ($netid, $sitename, $sector) = @_;
  $DAILY_HEAD= "Date       Total";
 
  $,="\t";    #Output Field Separator

$query_string="netid=$netid-$sitename-$sector&report_type=Daily+Totals
+&market=PHL";
$query_string =~ s/[=&]/ /g;
@query_string=split(' ', $query_string);
$network_element=$query_string[1];
$report_type=$query_string[3];
$MARKET=$query_string[5];

@network_element=split(/-/, $network_element);
$id=$network_element[0];
$name=$network_element[1];
$sector=$network_element[2];
######################################################

$DAILY_PATH="/data1/$MARKET/reports";
$BUSY_PATH="/data1/$MARKET/bhreports";
$PKD_PATH="/data1/$MARKET/pkdreports";
$MISC_PATH="/data1/$MARKET/miscreports";
$MDG_PATH="/data1/$MARKET/mdgreports";

$cellname="$network_element[1] Sector $network_element[2]";
$search_string="\Q($id)";

print "$cellname\n";

    @all_readable_files=grep !/\.Z/, `ls -t $DAILY_PATH | head -95`;
    #@all_readable_files=grep !/\.Z/, `ls -t $DAILY_PATH`;
    print "$DAILY_HEAD\n";
    foreach $file(@all_readable_files) {
        chomp($file);
        @date = split /_/, $file;
          $date = $date[0];
        
        open(FILE, "$DAILY_PATH/$file");
        @DATA=<FILE>;
        @result = grep /$search_string/, @DATA;
        $hits = grep /$search_string/, @DATA;
        for ( $i = 0; $i < $hits; $i++ ) {
        @_=split('    ', $result[$i]);
        chomp @_;
        print $date[0],$_[7]; 
        print "\n";
        }
        close FILE;

Now, the only problem is that $cellname isnt printing correctly. This is what i should get as the titles:

0040 Sector 1
0040 Sector 2
5228 Sector 1
5228 Sector 2
5458 Sector 1
0473 Sector 3
5335 Sector 1
5335 Sector 2
0267 Sector 3
0314 Sector 1
0314 Sector 3
5268 Sector 1
5268 Sector 2


This is what i do get

0040 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
5228 Sector 1
0040 Sector 2
0040 Sector 2
0040 Sector 2

I know the netid's that the sunroutine gets are being processed correctly, its the titles that are causing the problems. 0040 Sector 1 0040 Sector 2 5228 Sector 1 5228 Sector 2 5458 Sector 1 0473 Sector 3 5335 Sector 1 5335 Sector 2 0267 Sector 3 0314 Sector 1 0314 Sector 3 5268 Sector 1 5268 Sector 2

Kilinrax - I also did try a for loop, but it took forever to run. this one takes about 2 mins, the other.. i dont even it remember finishing.

arturo - to answer your question: the $index was used for the forloop, i just forgot to delete it, thanks much.

I hope this helps.

Dipul

[arturo]

This is a followup to tilly's post, above. One place to look in this subroutine -- which is using globals -- is at your variables. The output suggests that the value set in a previous call to the sub is being re-used in subsequent calls. If you use my to declare those variables in the subroutine, this can't happen, and moreover, if anything goes wrong and you inadvertently assign undef to one of them, you'll get a warning (provided you turn on -w or use warnings. This will help you track down errors.

Installment #157145 in the 'we were really serious when we said "use strict and warnings" series' =)

Philosophy can be made out of anything. Or less -- Jerry A. Fodor

[kilinrax]

Could could make this code a lot cleaner. Notably, that mess of splits and data assignments at the top, using `ls` when you should be using readdir, and scoping of variables.

Here's what i came up with after five minutes:

sub PRINT_INFO {
    my ($netid, $sitename, $sector) = @_;
    my $DAILY_HEAD = "Date       Total";
 
    $,="\t";    #Output Field Separator
 
    my $report_type = 'Daily Totals';
    my $MARKET = 'PHL';
 
    my $DAILY_PATH = "/data1/$MARKET/reports";
    my $BUSY_PATH = "/data1/$MARKET/bhreports";
    my $PKD_PATH = "/data1/$MARKET/pkdreports";
    my $MISC_PATH = "/data1/$MARKET/miscreports";
    my $MDG_PATH = "/data1/$MARKET/mdgreports";
 
    my $cellname = "$sitename Sector $sector";
    my $search_string = "\Q$netid";
 
    print "$cellname\n";
 
    opendir DIR, $DAILYPATH;
    my @all_readable_files = grep { ($_ ne '.') && ($_ ne '..') && ($_
+ !~ /\.Z//) } readdir DIR;
    closedir DIR;
 
    print "$DAILY_HEAD\n";
 
    my $file;
    foreach $file (@all_readable_files) {
        my $date = substr ($file, 0 , (index $file, '_'));
 
        open(FILE, "$DAILY_PATH/$file");
        @DATA=<FILE>;
        @result = grep { (index $_, $search_string) > -1 } @DATA;
        for ( $i = 0; $i <= $#results; $i++ ) {
            my $info = (split('    ', $result[$i]))[7];
            print $date , $info . "\n";
        }
        close FILE;
    }
}

[arturo]

This is potentially a tough one to solve without the data, and the subroutine. Are their duplicate lines in your data? What's $index doing in your script? I don't see it being used anywhere in the code you've posted.

A main worry here is that you may not have the right kind of data structure: you've got three arrays, but I'm not sure about how the data in them are related. You loop over one array, and pass elements from different arrays to the subroutine. Could you perhaps say more about your data structure (e.g. what data is contained within a single record) ? I would guess, from your description, that an array of array references (which is how one implements a two-dimensional array in Perl) would help to solve your problem.

an aside concerning that $index variable : foreach is actually a synonym of for as far as Perl is concerned, but you seem to be mixing up the two ways of using them to iterate over an array (by iterating over the indices of the array, or by iterating -- sans index -- over the array)

Philosophy can be made out of anything. Or less -- Jerry A. Fodor