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 > -- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] http://learn.perl.org/