Re: readdir formulated badly? gives wrong count



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, rea...@xxxxxxxxxxx wrote:
Thanks for taking time to respong and go clear thru the script bit by
bit.

kens <kenslate...@xxxxxxxxxxx> 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

.



Relevant Pages

  • Re: Scope
    ... > I a new Perl programmer dealing with issues of scope. ... > write a simple library using strict. ... The script that runs the library ... The usual way to implement libraries is to put the stuff into modules, ...
    (perl.beginners)
  • Re: readdir formulated badly? gives wrong count
    ... The `warnings' part is clear enough but doesn't the -w flag do something ... Not to be obtuse but why is declaring in smallest scope important ... This script is a helper script, so why is declaring variables like: ... function or internal loop or other more limited scope. ...
    (perl.beginners)
  • Re: same variable defined twice
    ... I've tried the following script and it works fine: ... script if using "use strict"? ... Lexical variables are intended for temporary use in a limited scope. ... You should always enable lexical warnings as well as strictures. ...
    (perl.beginners)
  • Re: Reading a line from an input file.
    ... It's not strictly a matter of scope, at least, not as you would ... input into a while loop, ... Put the whole script in a subshell: ...
    (comp.unix.shell)
  • Re: Newbie Question on Variables
    ... scope of a For Each loop will not be accessible to Tasks *outside* the loop. ... In the Script Task Editor, ... access to these variables anywhere in my package, ...
    (microsoft.public.sqlserver.dts)