Question on loops and return values

[delfuego]

I have a script below that is supposed to read in a list of microsoft
servers, and output the disk space (current and last), dates
(beginning and ending), and compute space growth in value and
percentage; to output file. The question is this: how do I get the
correct looping going on in my for loops (one is not getting the
machines, the other is not getting the drive letters; as well as any
other problems you can point out. The files are listed below the
scriipt, although I plan to write out two additional columns, growth
value and growth rate.
ALL help is appreciated.
Thanks,

James

<script beow inline>

use lib "$ENV(HOME)/site/lib"; # occasionally perl whines about not
in a lib
no lib ".";
use warnings;
use Time::localtime; # needed for tm and using that to get dates
use Time::tm;
use Win32API::Resources; # needed for drives and disk space

our @drive = Win32API::Resources::GetDrives();
our %space = Win32API::Resources::GetDriveSpace("$let:\\");
our $file = (<SD>,"growth.cvs"); # output data file
our $file2 = (<SD>,"machines.cvs"); # input data file
our $let = ['A-Z']; # character value for drive letters
our $gvalue = 0; # difference, should be positive
our $curr = 0; # current disk space value
our $last = 0; # last disk space value
our $grate = 0; # growth rate percentage
our @line = ""; # was going to be used for file looping
our $edate = ""; # end date for run
our $sdate = ""; # start date for run

# Mind you, we still need to loop this through machines in a list, but
# this will supposedly loop through the letters in the GetDrives
returns
open (file2,"+<"); # open machines input file for read
while <file2> # loop while not eof in machines.cvs
{
foreach $machine (<file2>)
{
print "The following are valid disk drives: @drive\n";
foreach $let (@drive)
{
&space();
&growth();
}
}
}

close file2;

sub space()
# This will return the space details available from ShowKeys in three
line format
{
Win32API::Resources::ShowKeys("Drive Space:", 2, \%space);
}

sub growth()
# Reads in current value from file growth.cvs to be the value for
last, then
# the %DRVSpace becomes current's value. After this, the dates are
set and
# we start computing the growth, giving growth difference and
percentage.
# Finally, all details are written to the appended file.
{
open file or die "Cannot open data file for read\n";
while <file>
{
$sdate = (read(file,"$junk," ",$edate," ",$junk," ",$junk) =
split);
chop ($sdate);
$last = (read(file,"$junk," ",$junk," ",$junk," ",$curr) = split);
chop ($last);
}
close file;
# we have the last and sdate variables, now we need the others
my $edate = Time::tm(localtime(){$4,$5,$6});
$curr = %DRVSpace;
$last = eval { $curr / 5 }; # bogus value for testing, should be
file($curr)
# now we sart computing
$gvalue = eval { $curr - $last }; # growth rate difference
$grate = eval { ($gvalue / $last) * 100 }; # growth rate percentage
# time for a screen dump
print "Date: $edate";
print "Last: $last\tCurrent: $curr\tGrowth: $gvalue\tGrowth Rate:
$grate\n";
# now to dump everything to the file
open (file,"+>> ") or die "Cannot open data file for write\n";
while <file>
{
print (file,"\n$sdate $edate $last $curr");
}
close file;
}

<files>

machines.csv
---------------
snoopy
linus
charlie
lucy
pigpen
freida

growth.csv

 

[Jim Keenan]

I have a script below that is supposed to read in a list of microsoft
servers, and output the disk space (current and last), dates
(beginning and ending), and compute space growth in value and
percentage; to output file. The question is this: how do I get the
correct looping going on in my for loops (one is not getting the
machines, the other is not getting the drive letters; as well as any
other problems you can point out. The files are listed below the
scriipt, although I plan to write out two additional columns, growth
value and growth rate.
ALL help is appreciated.
Thanks,

James

Am I correct in suspecting that this is a script that someone else
originally wrote and you have to fix? There's a lot of
less-than-optimal code below (e.g., use of 'our' where 'my' would almost
certainly suffice).

<script beow inline>

use lib "$ENV(HOME)/site/lib"; # occasionally perl whines about not
in a lib
no lib ".";
use warnings;
use Time::localtime; # needed for tm and using that to get dates
use Time::tm;
use Win32API::Resources; # needed for drives and disk space

our @drive = Win32API::Resources::GetDrives();
our %space = Win32API::Resources::GetDriveSpace("$let:\\");

Here a USENETtiquette problem arises. In order for us to troubleshoot
your code, we need to be on Windows, because the Win32API modules only
work on that OS. If your problem is a Windows-specific problem, then
you should probably post it on a Windows-specific list such as
(e-mail address removed). But I'm not reading this on Windows,
so I can't test your code. My hunch is that your problem is *not*
Windows-specific, but is located in how to read directory and file
information.

Wherever you post, you should try to post the smallest possible amount
of code needed to illustrate the problem. Most of the people who read
this list don't have enough time to work through all of it --
particularly if it requires a particular OS. If you try to post the
smallest possible amount you will, in the process, probably locate your
error yourself.

our $file = (<SD>,"growth.cvs"); # output data file
our $file2 = (<SD>,"machines.cvs"); # input data file
our $let = ['A-Z']; # character value for drive letters

[snip balance]

Jim Keenan

 

[Tad McClellan]

<script beow inline>

Please show *real code*!

use lib "$ENV(HOME)/site/lib"; # occasionally perl whines about not

^ ^
^ ^ {HOME}

our $file = (<SD>,"growth.cvs"); # output data file
our $file2 = (<SD>,"machines.cvs"); # input data file

Where is the SD filehandle open()ed?

Why are you using package variables instead of lexical variables?

our $let = ['A-Z']; # character value for drive letters

You never make use of this variable, and it almost certainly
is not what you think it is.

It is a reference to an anonymous array with 1 element in it.

our @line = ""; # was going to be used for file looping

How may elements are you expecting to be in @line right here?

Were you expecting zero elements, an empty array?

That isn't what you are getting there...

open (file2,"+<"); # open machines input file for read

You are opening for read AND write, the comment is a trick!

Did you mean to put a filename in there somewhere?

You should use UPPER CASE for filehandles.

You should always, yes *always*, check the return value from open().

while <file2> # loop while not eof in machines.cvs

Syntax error.

Show *real code*!

foreach $machine (<file2>)

You are reading from the _same_ filehandle in 2 different places
in your code. Is that what you want to do?

The while loop (if it compiled) would only execute one time.

The foreach provides list context, so it will slurp to the end of file.

chop ($last);

That is how we removed newlines 8 years ago. Where did you learn that?

Nowadays you should use chomp() to remove newlines.

$curr = %DRVSpace;

This most certainly does not do what you think it does.

The value of a hash in scalar context is not of interest to
a Perl programmer, it is only of interest to a perl programmer.

$last = eval { $curr / 5 }; # bogus value for testing, should be
$gvalue = eval { $curr - $last }; # growth rate difference
$grate = eval { ($gvalue / $last) * 100 }; # growth rate percentage

Can you tell us why you are using eval() there?

 

[Anno Siegel]

[...]

$grate = eval { ($gvalue / $last) * 100 }; # growth rate percentage

Can you tell us why you are using eval() there?

It's probably a misguided way of saying

$grate = ($gvalue / $last) * 100 if $last;

Anno

 

[Anno Siegel]

Anno Siegel ([email protected]) wrote on MMMMCXVI
September MCMXCIII in <URL:__
__ [...]
__
__ > > $grate = eval { ($gvalue / $last) * 100 }; # growth rate percentage
__ >
__ >
__ > Can you tell us why you are using eval() there?
__
__ It's probably a misguided way of saying
__
__ $grate = ($gvalue / $last) * 100 if $last;


Except of course that the latter can give you an "Illegal division by zero"
error, since numeric context ain't the same as boolean context.


$str = "700 00";
($gvalue, $last) =~ $str = /(\d+)\s+(\d+)/;
$grate = ($gvalue / $last) * 100 if $last;
__END__
Illegal division by zero

Yes, they're not equivalent, but "...if $last" is the far more common
idiom, and it does the job for numeric data. The original code is too
botched to make out what to expect for $last. Then again, the OP also
has

$gvalue = eval { $curr - $last };

so it looks like the author used eval for no good reason at all.

Anno

 

[Anno Siegel]

Anno Siegel ([email protected]) wrote on MMMMCXVI
September MCMXCIII in <URL:__
__ [...]
__
__ > > $grate = eval { ($gvalue / $last) * 100 }; # growth rate percentage
__ >
__ >
__ > Can you tell us why you are using eval() there?
__
__ It's probably a misguided way of saying
__
__ $grate = ($gvalue / $last) * 100 if $last;


Except of course that the latter can give you an "Illegal division by zero"
error, since numeric context ain't the same as boolean context.


$str = "700 00";
($gvalue, $last) =~ $str = /(\d+)\s+(\d+)/;
$grate = ($gvalue / $last) * 100 if $last;
__END__
Illegal division by zero

Yes, they're not equivalent, but "...if $last" is the far more common
idiom, and it does the job for numeric data. The original code is too
botched to make out what to expect for $last. Then again, the OP also
has

$gvalue = eval { $curr - $last };

so it looks like the author used eval for no good reason at all.

Oh, and... good to see you back.

Anno

 

[James]

Hi Jim,

Sorry to you and all other posters for my general usenet "Blurting". I
guess what I was dumping the code here for was in light of lacking the
ability to express the problem in theory. What is in general the
problem is maniipulating input and output through module functions,
namely Win32::Resources(GetDrives(), GetDriveSpace(), and ShowKeys()).
This was in fact a 'hacked' script, I was trying to use code originally
useful but directed toward tailor-made for my purposes. I will respond
more in-depth tonight.

Thanks again.

James

 

[James]

It was .. for some reason I had the misguided idea that each math
expression had to be in an eval. I don't know why.

 

[James]

Hi Tad,

Please show *real code*!
use lib "$ENV(HOME)/site/lib"; # occasionally perl whines about not
^ ^
^ ^ {HOME}

Tad thanks .. I did not catch that at all.

Where is the SD filehandle open()ed?

I have to apologize to everyone and thank them at the same time, that
was a horrible version of my non-working script that was posted. A new
one was posted today. Under a MS-blah header.
I do not believe in hindsight that it was ever opened.

Why are you using package variables instead of lexical variables?

This I will have to re-read on, I can't recall what the difference is
between them, but if I think I know what they are, I believe I was
using package variables because they were what I thought was necessary.

our $let = ['A-Z']; # character value for

drive letters
You never make use of this variable, and it almost certainly
is not what you think it is.

I thought it was used in a foreach for a loop control variable
associated with an array, but I could be wrong on that too. I was
trying to define a range of characters in a variable (which I guess
would have to be an array anyway).

It is a reference to an anonymous array with 1 element in it.
our @line = ""; # was going to be used for file looping
How may elements are you expecting to be in @line right here?
Were you expecting zero elements, an empty array?
That isn't what you are getting there...

I was expecting an empty array actually, or at least null values in
it.

You are opening for read AND write, the comment is a trick!
Did you mean to put a filename in there somewhere?
You should use UPPER CASE for filehandles.
You should always, yes *always*, check the return value from open().

I thought the + was to keep it from being clobbered..I would have to
put a filename in there I guess, I thought it was defined up top with
the filehandle. I have taken your advice and started using uppercase
for all filehandles. Why does the return value matter? Wouldn't it
just open or fail (die)?
I generally (now) use ((|| -or- or) die) after the open.

Syntax error.
Show *real code*!
You are reading from the _same_ filehandle in 2 different places
in your code. Is that what you want to do?
The while loop (if it compiled) would only execute one time.
The foreach provides list context, so it will slurp to the end of

file.

Man this script was a piece. I believe what I was trying to do was
loop through the machines listed by name in the file, does foreach only
provide list context?

That is how we removed newlines 8 years ago. Where did you learn that?
Nowadays you should use chomp() to remove newlines.

I am now using chomp(); I learned chop(); in an old NT book that had a
Perl script in it to do roughly the same thing.

This most certainly does not do what you think it does.
The value of a hash in scalar context is not of interest to
a Perl programmer, it is only of interest to a perl programmer.

So would it be $curr = /%DRVSpace to get the value of the key, or doyou
have to explicitly go through the (sort(keys /%DRVSpace);?

percentage
Can you tell us why you are using eval() there?

I thought eval was used for numeric expressions assigned to variables,
or maybe I'm confusing it with a bash statement?
Thanks for your time.

James

 

[Tad McClellan]

Tad thanks .. I did not catch that at all.

You should always enable warnings when developing Perl code!

warnings would have caught that mistake *for you*.

Take all the help you can get. Start your programs with:

use warnings;
use strict;

This I will have to re-read on, I can't recall what the difference is
between them,

See:

"Coping with Scoping":

http://perl.plover.com/FAQs/Namespaces.html

but if I think I know what they are, I believe I was
using package variables because they were what I thought was necessary.

I don't see anything that makes them necessary...

our $let = ['A-Z']; # character value for

drive letters
You never make use of this variable, and it almost certainly
is not what you think it is.

I thought it was used in a foreach for a loop control variable
associated with an array, but I could be wrong on that too.

$let contains a _reference_ to an anonymous array.

I was
trying to define

The array referenced by $let has ONE element in it,
a 3-character string "A", "-", "Z".

a range of characters in a variable (which I guess
would have to be an array anyway).

If you want the anon array to contain 26 elements then:

my $let = ['A' .. 'Z'];

but you have no need for an anon array at all, you can just
use the 26-element list right in the foreach

foreach my $letter ( 'A' .. 'Z' ) {
...
}

I was expecting an empty array actually, or at least null values in
it.

It has ONE element in it. The value of the one element is the empty string.

If you wanted zero elements, then either of these will do it:

my @line; # contains empty list by default

my @line = (); # be unnecessarily explicit about the empty list

I thought the + was to keep it from being clobbered..

It cannot be clobbered since your program never writes to that filehandle.

Why does the return value matter?

Because it tells you whether you actually got what you asked for or not.

Wouldn't it
just open or fail (die)?

No. (so you should put the "or die" stuff in there yourself)

If you read from an unopen()ed filehandle, you get an immediate EOF,
it looks like the file is empty.

If you write to an unopen()ed filehandle, your output goes in
the bit bucket, ie. is lost.

warnings will complain for either of those cases, but you should
test the return value yourself so that you can exit more gracefully.

Man this script was a piece.

Errr, right.

Have you seen the Posting Guidelines that are posted here frequently?

does foreach only
provide list context?

Yes.

Why do you ask?

Do you think you need something other than list context?

So would it be $curr = /%DRVSpace to get the value of the key,

What happened when you tried it?

 

[Ben Morrow]

Quoth (e-mail address removed):

Why does the return value [of open] matter?

Because it tells you whether you actually got what you asked for or not.

Wouldn't it
just open or fail (die)?

No. (so you should put the "or die" stuff in there yourself)

Unless you

use Fatal qw/open/;

in which case it will (but you should catch that exception with eval so
you can handle the error decently).

Ben

 

[Michele Dondi]

Hi Jim, [snip]
Thanks again.

James

Please provide some context quoting the relevant parts of the message
you're answering to. In *most* situations it's the Right Thing(TM) to
do. Even if your followup is addressed specifically to "Jim", if it is
posted here, rather than mailed personally to him, it is to be
implicitly intended that you're doing so for the benefit of everybody
who could read it, or at least it is reasonable to assume so.


Michele

 

[Michele Dondi]

It was .. for some reason I had the misguided idea that each math
expression had to be in an eval. I don't know why.

Please provide some context.


PS: I saw a similar line of code in a script written by a friend of
mine. I wonder if there could be a common source/explanation for this
oddity...


Michele