[EMAIL PROTECTED] wrote:
> 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'?
>
>
http://www.perlmonks.org/?node=use%20strict%20warnings%20and%20diagnostics%20or%20die
>>> 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.
>
That's not Perl, that's programming. It's good practice which, in
addition to keeping things more organized, helps to eliminate problems
down the road and optimize memory consumption.
>> 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.
>
>
The for loop won't execute if it has nothing through which to iterate.
On my system the for won't execute with an empty list, but will once
when the array is undefined. Changing the above to 'if
(defined(@DirContent)) {' would be a bit clearer.
>>> 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/
