Been away from the computer for a while, and I see that some of your
questions have been addressed, but I will add a few comments.
On Dec 1, 10:44 am, [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'?
>
see the documentation (perldoc strict) for more explanation.
I like it because it keeps me from shooting myself in the foot when I
misspell a variable name. If forces you to define all variables.
For example:
without strict I could say:
$this = 20;
print "$thus\n";
and no error is generated.
Withh strict:
use strict;
my $this = 20;
print "$thus\n";
An error message indicates that variable $thus is unknown.
This can help find subtle errors.
>
>
> >> 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.
>
Look at the following article on scope (Look at When to Declare
Variables):
http://www.developer.com/lang/perl/article.php/3323421
http://www.developer.com/lang/perl/article.php/10940_3323421_2
> > 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.
My point was that the for loop does this check for you. The if is
redundant.
>
> >> 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 a file happened to contain a number and text, it would also match,
and it mat break your code.
> >> ## 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?
>
Easier to read, you don't have two adjacent slashes.
> >> 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?
Your method will not list the full path to the script unless the full
path is given on the command line.
NOTE: I have a useless use of quotes, should have been:
my ( $myscript ) = fileparse( $0, '' );
HTH, Ken
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
