On Aug 3, 6:54 am, [EMAIL PROTECTED] (Mihir Kamdar) wrote:
> 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.
Most of the things I would point out have already been mentioned in
this thread but here are a couple I haven't noticed.
> #!/usr/bin/perl -w
The -w switch has now been replaced with
use warnings;
Unless compatibility with old versions of Perl is an issue you should
use the new version.
You should also
use strict;
> unless (exists $times{$file}){
Nothing ever sets anything into %times so that condition is always
false.
Unless you are actually expecting %times to contain false values then
it's more idiomatic to omit the exists()
> my $line;
You are still have a very minor case of premature declaration.
> 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;
If you are not bothering to checking the return value of close() then
you may as well simply omit the explicit close() if the handle is
about to go out of scope anyhow. This applies to all close() and
closedir() in your script.
> }
BTW: Your indentation is a bit random. In small programs like this
it's not much of an issue.
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
