interpreting script

[Shawn]

Hi,

I'm having a hard time interpreting a portion of a perl script (below). I
think its looking for the file pattern then determining the age and removing
it if its over 30 days old. The line with >30 has kind of got me. Any help
would be greatly appreciated.

@down_file_list = `/bin/ls *.downloaded *.kl2_download | grep -v
"_ppsd.zip" 2> /dev/null`;

foreach $down_file (@down_file_list) {
chop($down_file); #remove newline
if (-M $down_file > 30 ) {
print "Removing file $down_file in $search_dir/$home_dir\n";
system("/bin/rm -f $down_file");
}
}

 

[Walter Roberson]

:I'm having a hard time interpreting a portion of a perl script (below). I
:think its looking for the file pattern then determining the age and removing
:it if its over 30 days old. The line with >30 has kind of got me.

: if (-M $down_file > 30 ) {

-M is documented in perlop :

-M Script start time minus file modification time, in days.

So the check is for whether the file was modified more than 30 days
before the script started running.

 

[Ben Morrow]

More effective (mind you, though: I'm *not* saying these are the best
WTDI) code may be as follows:

for (grep /\Q_ppsd.zip/ && -M >30,

Err... this is wrong (though perhaps you just don't know what `grep
-v` does):

for (grep -M > 30 && ! /\Q_ppsd.zip/,

or

for (grep { -M > 30 and not /\Q_ppsd.zip/ }

as that expr is quite complicated enough to deserve a block of its
own (I find '... and not ...' much less confusing than 'not ... and
....', and I don't care about the unnecessary stat()s).

<*.downloaded *.kl2_download>) {
unlink $_ or
warn("Can't remove `$_': $!\n"), next;
print "Removing `$_'\n";

Surely the print should come before the unlink? Don't put "\n" on the
end of warn and die messages: it suppresses the line number info. Set
$\ rather than put "\n" on the end of every print statement.

If you don't want to print if the unlink fails, I would have written

unlink ?
warn "can't unlink '$_': $!" :
print "removed '$_'";

}

or perhaps:

unlink $_ and
!print "Removing `$_'\n" or
warn "Can't remove `$_': $!\n" for
grep /\Q_ppsd.zip/ && -M >30,
<*.downloaded *.kl2_download>

Ouch! What's that ! there for? Assuming the print will always
succeed, this evaluates as

(unlink and 0) or warn

which means that the warn will always happen. Even without that, it's
clearer as

unlink ? print : warn

; and it could have done with more friendly formatting:

unlink ? print "removed $_" : warn "can't unlink $_: $!"
for grep { -M > 30 and not /\Q_ppsed.zip/ }
glob "*.downloaded *.kl2_download";

Ben

 

[David K. Wall]

[snip...] Even without that, it's clearer as

unlink ? print : warn

; and it could have done with more friendly formatting:

unlink ? print "removed $_" : warn "can't unlink $_: $!"
for grep { -M > 30 and not /\Q_ppsed.zip/ }
glob "*.downloaded *.kl2_download";

I don't like any of the ways you all have written it.

for my $file (glob "*.downloaded *.kl2_download") {
next unless -M $file > 30 and $file !~ /\Q_ppsed.zip/;
unlink $file or do {
warn "Can't unlink '$file': $!";
next;
};
print "Removed '$file'\n";
}

It's a bit more verbose, but easier (at least for me) to read.

 

[Ben Morrow]

In fact this is what I want. Pease read more carefully and you'll
discover that...

^^^^^^

...it will always happen *if* unlink() succeeds. Won't it?

No. The print will happen iff the unlink succeeds, but the warn will
always happen. Consider:

1. unlink fails

(0 and <not evaluated>) or warn
=> 0 or warn
=> warn

2. unlink succeeds

(1 and 0) or warn
=> 0 or warn
=> warn

I am slightly puzzled as to what you even *thought* you meant to
write?

Ben

 

[Michele Dondi]

I'm having a hard time interpreting a portion of a perl script (below). I
think its looking for the file pattern then determining the age and removing
it if its over 30 days old. The line with >30 has kind of got me. Any help
would be greatly appreciated.

Short answer: Yes, sort of!

[OT wrt you request/subject line:]

Maybe you had a hard time interpreting it because it is an especially
bad example of Perl code. Bad programming habits/techniques include
but are probably not limited to:

@down_file_list = `/bin/ls *.downloaded *.kl2_download | grep -v

^^^^^^^^^^^^^^^ ^^^^^^^ ^^^^

1. (Probably) not using strict

2. Useless use of an external cmd

3. Double useless use of an external cmd

"_ppsd.zip" 2> /dev/null`;

foreach $down_file (@down_file_list) {

^^^^^^^^^^

4. (Definitely) not using strict

chop($down_file); #remove newline

^^^^^^^^^^^^^^^
5. Use of old-fashioned chop() instead of more modern and secure
chomp().

if (-M $down_file > 30 ) {
print "Removing file $down_file in $search_dir/$home_dir\n";
system("/bin/rm -f $down_file");

6. Yet another usless use of an external cmd.

7. (Overall multiple selections happen at different places whereas
they *could* (easily) be done at the same time.

More effective (mind you, though: I'm *not* saying these are the best
WTDI) code may be as follows:

for (grep /\Q_ppsd.zip/ && -M >30,
<*.downloaded *.kl2_download>) {
unlink $_ or
warn("Can't remove `$_': $!\n"), next;
print "Removing `$_'\n";
}

or perhaps:

unlink $_ and
!print "Removing `$_'\n" or
warn "Can't remove `$_': $!\n" for
grep /\Q_ppsd.zip/ && -M >30,
<*.downloaded *.kl2_download>

[both untested]


HTH,
Michele

 

[Michele Dondi]

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Please note that I prepended this caveat!

Err... this is wrong (though perhaps you just don't know what `grep
-v` does):

I *do* know what -v does. Just missed that switch!

for (grep { -M > 30 and not /\Q_ppsd.zip/ }

as that expr is quite complicated enough to deserve a block of its
own (I find '... and not ...' much less confusing than 'not ... and

Fundamentally I agree. To be more precise, *as far as my personal
tastes are concerned*, I would tend to consider this a *limit* case,
in the sense that the expr is indeed complicated enough that it's not
very nice to have in for (...) but not enough to definitely "deserve"
a separate block.

Surely the print should come before the unlink? Don't put "\n" on the

Surely not "surely". I don't want to print if the unlink() fails. I
agree that there may be a feeble reason for you saying so, based upon
the choice of the message that *may* have better been "`$_' removed".

But then this doesn't really make a big difference and is not an issue
with perl *code*, I'd say!

end of warn and die messages: it suppresses the line number info. Set
$\ rather than put "\n" on the end of every print statement.

I *do* know what "\n" does (with die() and warn()). I use it if I
think it is useful, and I don't use it if I think it is not: simple,
isn't it?

Did you take into account the possibility that the end user of the
program may not be interested in those pieces of info, whereas he
would be content with knowing the file that caused the problem and a
reason why it happened?!?

If you don't want to print if the unlink fails, I would have written

You got it man!

unlink ?
warn "can't unlink '$_': $!" :
print "removed '$_'";

As usual it's a matter of personal taste: believe me, I like the ?:
operator and find it quite useful, especially when I can use its
return value wich is what I need in most cases. Definitely in this
particular case I would have favoured an if then else structure
construct instead.

Ouch! What's that ! there for? Assuming the print will always
succeed, this evaluates as

(unlink and 0) or warn

In fact this is what I want. Pease read more carefully and you'll
discover that...

which means that the warn will always happen. Even without that, it's

^^^^^^

....it will always happen *if* unlink() succeeds. Won't it?

; and it could have done with more friendly formatting:

unlink ? print "removed $_" : warn "can't unlink $_: $!"
for grep { -M > 30 and not /\Q_ppsed.zip/ }
glob "*.downloaded *.kl2_download";

As of my cmts above, I understand your concernings with the pieces of
code I proposed, especially the actual error (i.e. "ignoring -v"), but
to be fair I don't think this particular proposal of yours is in any
way more readable/terse than mine.

I don't like any of the ways you all have written it.

for my $file (glob "*.downloaded *.kl2_download") {
next unless -M $file > 30 and $file !~ /\Q_ppsed.zip/;
unlink $file or do {
warn "Can't unlink '$file': $!";
next;
};
print "Removed '$file'\n";
}

It's a bit more verbose, but easier (at least for me) to read.

I partially agree. Of course I wrote the code in the previous post as
I would have written it if it were to be included in a program of
mine.

In particular I do not have any problem using an explicit variable in
for cycles, but if the body of the cycle is short enough I prefer to
use the implicit one instead. And in this case I would consider it
short enough.

One thing about your proposal that I really do *not* like is the
(IMHO) unnecessary do block. But that is only MHO...


Michele

 

[Michele Dondi]

...it will always happen *if* unlink() succeeds. Won't it?

No. The print will happen iff the unlink succeeds, but the warn will
always happen. Consider: [snip simplified example]
I am slightly puzzled as to what you even *thought* you meant to
write?

D'Oh! I stand corrected! I wanted to write:

unlink and
print "Removing `$_'\n" or
warn "Can't remove `$_': $!\n" for
grep !/\Q_ppsd.zip/ && -M >30,
<*.downloaded *.kl2_download>

In this case if unlink() succeeds, then print() happens and (unless
I'm very very unfortunate!) succeeds, so warn() doesn't happen. If
unlink fails, print() is not evaluated and warn() does happen.

So you should not be puzzled as to "what I even *thought* I meant to
write", because that is obvious! You should be puzzled as to why I
made my life so complex by thinking that the return value of print()
needed some make up: well the *only* feeble excuse for that (and the
*second* one in just a few days!) is that I was remembering a totally
unrelated situation in which *that* particular make up was needed to
work correctly with short circuiting of logical operators.

Ben, I'm so ashamed, especially of my arrogant tone. I feel you would
have had all the rights to respond quite more harshly...


Michele