Thanks for taking time to respong and go clear thru the script bit by
bit.
kens <[EMAIL PROTECTED]> writes:
[...]
> # The following lines are your friend
> use strict;
> use warnings;
I've seen that before but didn't understand why.
The `warnings' part is clear enough but doesn't the -w flag do something
similar? But why `strict'?
>> if(!$ARGV[1]){
>> usage();
>> print "Too few arguments.. exiting\n";
>> exit;
>>
>> }
>>
>> if($ARGV[2]){
>> usage();
>> print "Too many arguments... exiting\n";
>> exit;}
>>
>> use Cwd;
>> my $Cwd = getcwd;
>>
>> use File::Copy;
>>
>
> Define variables in the smallest scope
> Plus only define variables you use, otherwise it can get confusing
>> my ($HiNum, $Copied, $TrgDir, @DirContent, $Cnt, $Line);
>> $TrgDir = shift;
>> $HiNum = 0;
>> $Copied = 0;
Is there something there, I don't use? I'm not seeing which one.
[...]
> To declare in smallest scope:
> my @DirContent = grep { /^[0-9]/ } readdir($DIR);
>> @DirContent = grep { /^[0-9]/ } readdir(DIR);
Not to be obtuse but why is declaring in smallest scope important
here? I know that is commonly offered as advice but I don't think I
have seen any explanation as to why.
This script is a helper script, so why is declaring variables like:
my $var = value
in the main part of the script a bad thing?
That already limits the scope to this script which itself seems
unnecessary. Like I could have just said:
$var = value in the main part and been ok, I used `my' for the
unlikely event I someday write something that references this script
like a function to some other script by slurping it. I don't know
enough perl to see why I need to make that declaration inside a
function or internal loop or other more limited scope.
> This test add nothing. The for loop will fail if there is no content
>
>> if ($DirContent[0]){
Checking to make sure there are in fact numbered files already in
TrgDir. As often as not it is a newly created and empty directory.
If there aren't numbered files alread in it then no need for the for
loop. So testing for value of $DirContent[0]) was supposed to bypass
the for loop if it were empty of value.
>> for(@DirContent){
>
> Do these files only contain numbers?
Yes. Note the argument to grep earlier /^[0-9]/.
I guess that should have been /^[0-9]+/ but the directories to be
copied from are populated by NNTP which always uses numbering.
>> ## If we already have numbered files, Start our counter with
>> ## the highest one
>> if($HiNum){
>> $Cnt = $HiNum;
>> }
Then the above test (using the result of the for loop if not bypassed)
is supposed to decide if we need to increase Cnt so as not to clobber
any existing files. If HiNum is still zero then there weren't any in
the TrgDir and no need to bump the Cnt up to protect them.
>>
>> while(<>){
>> chomp;
>> $Cnt++ ;
>> ## The first if clause here is unnecessary now... to be removed
>> if($TrgDir){
>> ## Add the correct path if our line doesn't start with '/'
>
> Use different delimiters if searching for a slash
Is that faster or somehow better than escaping the slash?
>> if(!/^\//){
>> $Line = $Cwd ."/". $_;
[...]
>> ## Report how many files were copied
>> print " <$Copied> Files copied to:\n $TrgDir\n\n";
>
>> print " ".CountTrgDir()."\n";
>
> should be closedir($DIR);
>> close(DIR);
> The main problem, I believe, is that you are using a stale directory
> handle. closedir (in main) and opendir here.
Ha... and that was the problem... turns out it doesn't need to be
moved. Once the typeo is corrected to closedir(DIR); it works, with
no reopen.
[...]
> use File::Basename;
>
>> sub usage {
>
> # use this line to get the name of your script
> # perldoc File::Basename
> my ( $myscript ) = fileparse( "$0", "" );
[...]
>> Usage: \`$myscript TARGETDIRECTORY SOURCEFILE'
Somehow in the course of many edits these lines up near the top were
lost. I insert them in most of my homeboy scripts for use in the
usage function:
my $myscript;
($myscript = $0) =~ s:^.*/::;
Is it still better to pull in a whole new module
(`use File::Basename;') to do that?
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
