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) ;
}

Reply via email to