On Thursday 17 Dec 2009 17:07:19 Simphiwe Mkhize wrote:
> Please Help
> 
> I have multiple folders want to read trough each folder and search for
>  certian file. I have code which is working fine but does not do what I
>  want. I dont want to hard code path as it read different folder within

You can input it from the command-line using @ARGV:

<<<<<<<<<<<<
my ($directory_to_search) = @ARGV;

# Do something with $directory_to_search
>>>>>>>>>>>>

Now a few comments on your code:

1. You should add "use strict;" and "use warnings;" at the top and fix all 
problems that they report.

> 
> sub get_phone_log {
> #--------------------------------------------------------------------------
> ---------------- 
> $p_dir="D:/43895" ;

Use my here.

>  print  $p_dir;

You probably want:

<<<<<<
print $p_dir, "\n";
L>>>>>

here instead.

> opendir(DIR, "$p_dir") or die "cant open directory: $!\n";
> 

Use lexical dir-handles. Kudos for the or die.

> while(defined($folder = readdir(DIR))) {
>   if (length $folder > 5)
>   {
>     print $folder;

Again "\n";

>   }
>    if (substr($folder,0,8) eq 'PHONELOG') {
>         &print_out_contents;
>         }
>     }

This is more idiomatically written as:

if ($folder =~ /\APHONELOG/)
{
}

And please don't use leading ampersands in subroutine calls:

http://www.perlfoundation.org/perl5/index.cgi?subroutines_called_with_the_ampersand

If it's an OS with case-insensitive filenames, you should use the /i flag:

 ... =~ /\Aphonelog/i

> 
> closedir(DIR) ;
> 
> #--------------------------------------------------------------------------
> ---------------- }
> 
> sub print_out_contents {
> #--------------------------------------------------------------------------
> ---------------- 
> my $phonelog = "$p_dir/$folder";

Please don't use such "#-----------------------" comments before and after the 
beginning of the block. They make the code mcuh harder to follow.

> 
> open  (P1FILE, "$phonelog") or die "can't open $phonelog: $!";

Use lexical filehandles, and three args open:

http://stackoverflow.com/questions/1479741/

<< "$phonelog" >> is probably better written as just << $phonelog >>.

> 
> flock (P1FILE, LOCK_SH)     or die "can't lock $phonelog: $!";
> 
> while (<P1FILE>)

It's better to do:

<<<
while (my $line = <$p1file>)
{
>>>

instead and use $line instead. $_ can be destroyed way too easily.

> {
>     chomp;
>     @linex = split(/\|/);
>     &plfields;
> 
>      &build_days;

@linex is a package variable - it should be a lexical - declared with my. 
Either pass it to the functions by reference or declare them after its 
declarations.

And don't use call-with-ampersand

> }
> 
> close   (P1FILE);
> 
> #--------------------------------------------------------------------------
> ---------------- }
> 
> 
> #--------------------------------------------------------------------------
> ---------------- sub plfields {
> #--------------------------------------------------------------------------
> ----------------
> 
> $plcallername    = $linex[0];
> $plcallerphone   = $linex[1];
> $plcalleremail   = $linex[2];

This is better done as:

<<<<<<<<<<
my ($plcallername, $plcallerphone, $plcalleremail) = @lines;
>>>>>>>>>>

That way it is easier to insert a variable in between.

You should also use underscores, because several words concatenated like that 
are hard to read. 

> 
> $plsentdate      = $linex[3];
> $plsentdate_ccyy = substr($plsentdate,0,4) ;
> $plsentdate_yy   = substr($plsentdate,2,2) ;
> $plsentdate_mm   = substr($plsentdate,4,2) ;
> $plsentdate_dd   = substr($plsentdate,6,2) ;

This is better done using a regex with {4} , {2}, etc.

> 
> $pltime          = $linex[4];
> $plfilebase      = $linex[5];
> $pltext          = $linex[6];
> 

Again, use the list form.

And make these variables lexicals - using my.

> ($plfilebase_type,$plfilebase_num,$plfilebase_name) = split (/\_/,
>  $plfilebase);
> 

Why are you using /\_/ instead of /_/, or /\\_/? 

Regards,

        Shlomi Fish

-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
The Case for File Swapping - http://shlom.in/file-swap

Bzr is slower than Subversion in combination with Sourceforge. 
( By: http://dazjorz.com/ )

-- 
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/


Reply via email to