looping questions

L

lerameur

Hello,

this is not a perl problem (maybe it is). The function does work, but
gave me error . there are 4 files into the traffic directory, the loop
goes into 24 possiblility, copies the four files and gives me 20
errors. I decided to add an IF statement to only copy if the file
exists. what is wrong with my IF loop ?
thanks

Ken



for my $hours ('00'..'23') {

$file23 = glob("$timestamp2$hours*") ;
print "$timestamp2: " ,$timestamp2, "\n";
print "file23: " ,$file23, "\n";
if ($file23 == 1){
system(`cp /input/fttr/traffic/$file23 /input/$Out_directory `);
}

$hours++;
}
 
J

Jim Cochrane

Hello,

this is not a perl problem (maybe it is). The function does work, but
gave me error . there are 4 files into the traffic directory, the loop
goes into 24 possiblility, copies the four files and gives me 20
errors. I decided to add an IF statement to only copy if the file
exists. what is wrong with my IF loop ?
thanks

You forgot to report the errors, or a good summary of the errors, in
your post.

Did you 'use strict;', 'use warnings;'?
for my $hours ('00'..'23') {

$file23 = glob("$timestamp2$hours*") ;

Odd var name.
print "$timestamp2: " ,$timestamp2, "\n";
print "file23: " ,$file23, "\n";
if ($file23 == 1){

Why are you testing whether $file23 equals 1? You probably want:
if ($file23){
system(`cp /input/fttr/traffic/$file23 /input/$Out_directory `);
}

$hours++;
}

--
 
G

Greg Bacon

: for my $hours ('00'..'23') {
: [...]
: $hours++;
: }

Your code is doing twice as much work as necessary to compute
the values of $hours:

1. Your for loop sets up a loop over '00' .. '23'
2. You increment $hours at the end of each iteration,
but the value is ignored.

You can remove the line with the increment.

: $file23 = glob("$timestamp2$hours*") ;

What if there's more than one match?

: if ($file23 == 1){
: system(`cp /input/fttr/traffic/$file23 /input/$Out_directory `);
: }

I don't see how the condition could ever be true because your
glob shouldn't match a file whose name is 1.

Perl's system operator wants a command to run, e.g.,

system("ls -l /etc/motd");

Backticks (``) also want a command. The value of a command
quoted with backticks is the output of running it. For example:

$ perl -e 'print `echo 1 + 2 | bc` * 4, "\n"'
12

You usually don't want to combine them as you've done because
that would run one command and then attempt to run its output
a second command, e.g.,

$ perl -e 'system `echo ls /`'

Consider the following improvements:

for my $hours ('00' .. '23') {
my $cmd = join " " => "cp",
"/input/fttr/traffic/$timestamp2$hours*",
"/input/$Out_directory";

system $cmd;
}

You could even condense your code to a single command:

my $base = "/input/fttr/traffic/$timestamp2";
system join " " => "cp",
map("$base$_*", '00' .. '23'),
"/input/$Out_directory";

Hope this helps,
Greg
 
X

xhoster

lerameur said:
Hello,

this is not a perl problem (maybe it is). The function does work, but
gave me error . there are 4 files into the traffic directory, the loop
goes into 24 possiblility, copies the four files and gives me 20
errors. I decided to add an IF statement to only copy if the file
exists. what is wrong with my IF loop ?
thanks

Ken

for my $hours ('00'..'23') {

$file23 = glob("$timestamp2$hours*") ;

You don't want to use scalar glob here. globs are kind of weird,
and it won't notice that it's argument changed until the is done iterating
through all of the files that matched its initial invocation (and has
returned undef once). Then it will look at its argument again, and start
iterating over that result. If you know that no more than one file will
match, or you only care about one of the matches in cases where there are
more than one, you could get it by using a list context:

my ($file23) = glob("$timestamp2$hours*") ;
print "$timestamp2: " ,$timestamp2, "\n";
print "file23: " ,$file23, "\n";
if ($file23 == 1){

That is wrong. It is not clear what exactly is wrong because we don't
what you want it to do.

Xho

--
-------------------- http://NewsReader.Com/ --------------------
The costs of publication of this article were defrayed in part by the
payment of page charges. This article must therefore be hereby marked
advertisement in accordance with 18 U.S.C. Section 1734 solely to indicate
this fact.
 
L

lerameur

: for my $hours ('00'..'23') {
: [...]
: $hours++;
: }

Your code is doing twice as much work as necessary to compute
the values of $hours:

1. Your for loop sets up a loop over '00' .. '23'
2. You increment $hours at the end of each iteration,
but the value is ignored.

You can remove the line with the increment.

: $file23 = glob("$timestamp2$hours*") ;

What if there's more than one match?

: if ($file23 == 1){
: system(`cp /input/fttr/traffic/$file23 /input/$Out_directory `);
: }

I don't see how the condition could ever be true because your
glob shouldn't match a file whose name is 1.

Perl's system operator wants a command to run, e.g.,

system("ls -l /etc/motd");

Backticks (``) also want a command. The value of a command
quoted with backticks is the output of running it. For example:

$ perl -e 'print `echo 1 + 2 | bc` * 4, "\n"'
12

You usually don't want to combine them as you've done because
that would run one command and then attempt to run its output
a second command, e.g.,

$ perl -e 'system `echo ls /`'

Consider the following improvements:

for my $hours ('00' .. '23') {
my $cmd = join " " => "cp",
"/input/fttr/traffic/$timestamp2$hours*",
"/input/$Out_directory";

system $cmd;
}

You could even condense your code to a single command:

my $base = "/input/fttr/traffic/$timestamp2";
system join " " => "cp",
map("$base$_*", '00' .. '23'),
"/input/$Out_directory";

Hope this helps,
Greg

Ok thanks

it was the if loop:
putting thiif s is good: ($file23){

k
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,769
Messages
2,569,582
Members
45,065
Latest member
OrderGreenAcreCBD

Latest Threads

Top