On 8/2/07, Chas Owens <[EMAIL PROTECTED]> wrote: > > On 8/2/07, Mihir Kamdar <[EMAIL PROTECTED]> wrote: > > > #!/usr/bin/perl > > > > my $child_path = > > > '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files/child' > > ; > > my $parent_path = > > > '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files' > > ; > > my $write_path = > > > '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files/output' > > ; > > my $line; > > This declaration is being done too soon. Variables should have the > smallest possible scope. $line is being used only with a while loop > below. It should be declared then. > > > my %hash; > > my @file; > > @file is never used > > > my $key ; > > my $value ; > > These variables are also being declared too soon. > > > my %times ; > > while () { > > Infinite loops are a bad idea. It is better to give it an exit > condition that can be triggered by a signal. The following code will > exit if the program receives either SIGINT or SIGTERM (control-c or > kill -15): > > my $continue = 1; > $SIG{INT} = $SIG{TERM} = sub { $continue = 0 }; > while ($continue) { > print localtime() . "\n"; > sleep 1; > } > > This also brings up another problem with your code: you don't sleep > inside the loop. This will cause your code to consume all available > CPU cycles. This is generally considered not a good idea. > > > opendir my $dh, $child_path or die $!; > > while (my $file = readdir $dh) { > > my $fname = "$child_path/$file"; > > next unless -f $fname; > > unless (exists $times{$file}){ > > opendir my $dh1,$parent_path or die $!; > > open (my > $IN_FILE,"<","$parent_path/$file") > > or die $!." Parent file not found for $parent_path/$file"; > > while ($line=readline($IN_FILE)) > > { > > my @cdr=split (/,/, $line) ; > > > > $hash{$cdr[2],$cdr[3],$cdr[6],$cdr[7]}=$line; #Add some more cdr key > fields > > if u want. > > This is not a good idea. The correct way to do this is either > $hash{$cdr[2]}{$cdr[3]}{$cdr[6]}{$cdr[7]} = $line; > or > $hash{"@cdr[2,3,6,7]"} = $line; > > > } > > close $IN_FILE ; > > closedir $dh1 ; > > open (my $OUT_FILE,">","$write_path/$file.out") or die > $!; > > while (($key, $value) = each %hash) > > { > > print $OUT_FILE $value; > > } > > close $OUT_FILE; > > } > > unlink("$parent_path/$file") ; > > unlink("$child_path/$file") ; > > } > > closedir $dh; > > } > > > > Please comment on the above code and tell me if it has any potential > > pitfalls. Right now I am able to test this for just few sample files. > But in > > real environment, it has to pick up millions of files, process them and > > write the output. > > > > Your comments will be highly appreciated. > > > > Thanks, > > Mihir > > > Hi,
I appreciate your comments and have tried to make changes as per your suggestions. Plz have a look at the below and comment if any further changes are required. This is turning out to be a good learning for me. #!/usr/bin/perl -w my $child_path = '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files/child' ; my $parent_path = '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files' ; my $write_path = '/home/user71/RangerDatasource/Customization/TelekomMalaysia/Scripts/Tests/cprogs/files/output' ; my %times ; my $continue = 1; $SIG{INT} = $SIG{TERM} = sub { $continue = 0 }; while ($continue) { opendir my $dh, $child_path or die $!; while (my $file = readdir $dh) { my $fname = "$child_path/$file"; next unless -f $fname; unless (exists $times{$file}){ my $line; my %hash; opendir my $dh1,$parent_path or die $!; open (my $IN_FILE,"<","$parent_path/$file") or die $!." Parent file not found for $parent_path/$file"; while ($line=readline($IN_FILE)) { my @cdr=split (/,/, $line) ; $hash{$cdr[2],$cdr[3],$cdr[6],$cdr[7]}=$line; #Add some more cdr key fields if u want. } close $IN_FILE ; closedir $dh1 ; open (my $OUT_FILE,">","$write_path/$file.out") or die $!; while (my($key, $value) = each %hash) { print $OUT_FILE $value; } close $OUT_FILE; } unlink("$parent_path/$file") ; unlink("$child_path/$file") ; } closedir $dh; sleep(2) ; }