Re: readdir formulated badly? gives wrong count

reader@xxxxxxxxxxx wrote:
Thanks for taking time to respong and go clear thru the script bit by
bit.

kens <kenslaterpa@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'?


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?




.

Relevant Pages

  • Re: readdir formulated badly? gives wrong count
    ... see the documentation (perldoc strict) for more explanation. ... 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: Logical problem with small script??
    ... > Wondered if anyone can throw some light on why this script won't act ... > offset 23 but I don't want it looping all the time. ... When this loop runs I get the warning: ... That eliminates the warnings message and makes the code shorter and easier ...
    (perl.beginners)
  • Re: Report in FM 6 or 7
    ... achieve this report format will have to be done via scripting. ... but they will simplify the script and Sort Order. ... End Loop ... The normal 'Dancer' field is used so that if there are more than one ...
    (comp.databases.filemaker)
  • Re: recreate database script not work
    ... I got a script which is supposed to regenerate database systax. ... REM gen_dbse_9.sql ... end loop; ...
    (comp.databases.oracle.server)
  • Re: How to set up a global variable in a sub-routine?
    ... the 'global variables' are more like constants ... ... single script, I need a way to tell it only once where the file is and ... So, if you construct a loop with a loop variable, you ... >programmers avoid global variables completely. ...
    (perl.beginners)