Creating a list of HASHes

[Patrick Paquet]

Hello everyone,

Not my first posting here, but the first one since I've read the
Posting Guidelines (hope I got it right)

I read the docs on 'use strict', but still don't quite understand why
I get "Can't use string ("Machine1") as a HASH ref while "strict refs"
in use at ... line 43".

Also ran it with 'Perl -w' but that did not return any info regarding
the "Can't use string" error, just some "Use of uninitialized value
at..." (which I don't get either btw).

All the subroutines return data as expected and the program executes
normally until line 43. This program is a bit lengthy to post in it's
entirety, so I snipped all irrelevant lines and modified the code so
that what is below replicates the error as I see it in the normal
version.

I am running on WinNT, perl version 5.005_02, ActivePerl(509). I know
it's old, but since I don't manage the script servers here I'm using
the same version as the server.

If there's anything missing from this, it's purely accidental, and I
will do my best to provide all necessary information.

Thanks for your time.


Patrick Paquet

###################################################
use strict;
use Win32::ODBC;
use File::Find;
use Win32API::Net;
use Win32::NetAdmin;
use Net:ing;
use Net::hostent;
use Socket;

my @ListDOM = qw/Machine1 Machine2 Machine3/; # This list is pulled
directly from the SAM on the PDC

my ($Machine, $ValeurCle, %MachineInfo, $CheckMachine, $Ping,
$ReverseLookup, $AdresseIP, $INTAdresseIP);
foreach $Machine (@ListDOM)
{
$CheckMachine = 0;
# %MachineInfo = &ReadSQL($ConnectionSQL,$Machine); # Commented to
allow
execution, otherwise runs OK
if ($MachineInfo{MachineName} ne $Machine) # Existe pas dans la
base
{
$CheckMachine = 1;
print "$Machine n'existe pas dans la base.($CheckMachine)\n";
}
$ValeurCle = 123456; #&GetValeurCle($Machine);
if ($MachineInfo{Valeur_CiPatches} != $ValeurCle)
{
$CheckMachine = 1;
print "$Machine: valeur cle differente.($CheckMachine)\n";
}
($Ping,$ReverseLookup,$AdresseIP)=("OK","OK","10.10.10.10");#&GetIP($Machine);
if ($MachineInfo{IP_address} ne $AdresseIP)
{
$CheckMachine = 1;
print "$Machine: adresse IP differente.($CheckMachine)\n";
}
if ($CheckMachine == 0)
{
print "Base a jour pour $Machine, machine suivante...\n\n";
}
else
{
print "Checks values on workstation...\n\n";

### ERROR REPORTED ON FOLLOWING LINE
&EcritureSQL(%$Machine);#Passes HASH to subroutine to write
to SQL db
###
} # Fin de la verification d'une machine

} # Fermeture de la boucle de verification des machines, toutes
les machines ont etes processe


sub EcritureSQL # Ecriture des infos dans la base SQL
{
# Doesn't do much at this point, until I get that error sorted out.
my ($Key, $Value, $Pause);
my %InfosMachine = %_;
while (($Key, $Value)=each %InfosMachine){print "Key:$Key \->
Value: $Value\n";}
$Pause = <STDIN>;
}

 

[Richard Gration]

### ERROR REPORTED ON FOLLOWING LINE

I'm not surprised! $Machine is an ordinary scalar variable. For this
syntax to work, $Machine must be a reference to a hash. Looking at your
code, are you sure you didn't mean to use %MachineInfo here instead of
%$Machine?

&EcritureSQL(%$Machine);#Passes HASH to subroutine to write
to SQL db
###
} # Fin de la verification d'une machine

} # Fermeture de la boucle de verification des machines, toutes
les machines ont etes processe
sub EcritureSQL # Ecriture des infos dans la base SQL {
# Doesn't do much at this point, until I get that error sorted out.
my ($Key, $Value, $Pause);
my %InfosMachine = %_;

This would give an error too: %_ is not a global variable. When you pass
a hash by value it is flattened to a list, you can just use

my %InfosMachine = @_; # <-- Note the '@' sign, not '%'

But better would be to pass by reference. Call the sub like this:

EcritureSQL(\%MachineInfo)

and then pick up the reference inside the sub with:

my ($InfoMachine) = @_;

and then the loop below is written as:

while ( ($Key,$Value) = each %$InfoMachine) {
print "Key:$key \-> Value: $Value\n";
}

while (($Key, $Value)=each %InfosMachine){print "Key:$Key \->
Value: $Value\n";}
$Pause = <STDIN>;
}

HTH
Rich

 

[A. Sinan Unur]

(e-mail address removed) (Patrick Paquet) wrote in

I ... don't quite understand why I get "Can't use string
("Machine1") as a HASH ref while "strict refs" in use at
... line 43". ....

my @ListDOM = qw/Machine1 Machine2 Machine3/; # This list is pulled
directly from the SAM on the PDC

my ($Machine, $ValeurCle, %MachineInfo, $CheckMachine, $Ping,
$ReverseLookup, $AdresseIP, $INTAdresseIP);

You are defeating the purpose of using my by putting all variable
declarations here. Declare your variables in the smallest scope that
they apply to. For example:

foreach $Machine (@ListDOM)

Instead, use

foreach my $Machine (@ListDOM)

{
$CheckMachine = 0;

my $CheckMachine = 0;

etc. You get the idea.

# %MachineInfo = &ReadSQL($ConnectionSQL,$Machine); # Commented to
allow execution, otherwise runs OK

Please do not use margin comments that wrap in newsreaders. It makes it
too much work to copy and paste your code to try to run it.

Also, do you know the difference between:

&ReadSQL($ConnectionSQL,$Machine);

and

ReadSQL($ConnectionSQL,$Machine);

If not, do not prefix you subroutine calls with &. (I have to admit,
however, that I am not familiar with such older versions of Perl, and maybe
there is a reason to use it in that environment).

### ERROR REPORTED ON FOLLOWING LINE
&EcritureSQL(%$Machine);#Passes HASH to subroutine to write
to SQL db

First, see my previous two comments.

Now, $Machine points an element of the array @ListDOM in each iteration
of the foreach above. Those elements are strings. You try to dereference
them as references to hashes. Why is the error message confusing?


Did you mean to pass the %MachineInfo hash to EcritureSQL? In that case,
btw, why pass the whole hash instead of just passing a reference to it?
Here is a small example:

use strict;
use warnings;

my %MachineInfo = (
'IP' => '192.168.1.1',
'Name' => 'Gateway',
'Role' => 'Who knows?',
);

EcritureSQL(\%MachineInfo);

sub EcritureSQL {
my $InfoMachine = shift;
foreach my $key (keys %$InfoMachine) {
print "Key: $key -> Value: $InfoMachine->{$key}\n";
}
my $pause = <STDIN>;
}

__END__

OTOH, when you want to dump data structures, you might just want to use
Data:umper:

use Data:umper;
sub EcritureSQL {
print Dumper(shift);
my $pause = <STDIN>;
}

I understand the dumping of key-value pairs was for debuging, but even that
purpose can be better served by using Data:umper.

 

[Brian McCauley]

&ReadSQL($ConnectionSQL,$Machine);

and

ReadSQL($ConnectionSQL,$Machine);

If not, do not prefix you subroutine calls with &. (I have to admit,
however, that I am not familiar with such older versions of Perl, and maybe
there is a reason to use it in that environment).

I believe it was required in Perl4.

--
\\ ( )
. _\\__[oo
.__/ \\ /\@
. l___\\
# ll l\\
###LL LL\\

 

[Patrick Paquet]

Hi,

I'm replying to the original message because both Richard Gration and
A. Sinan Unur have provided interesting and useful information, I hope
this isn't a major breach of group etiquette.

I should've probably said this in the original post, sorry, my bad.
What I am aiming to do in short, is this:
1. Fetch list of workstations from the domain controller,
2. Perform a series of tests on each machine,
3. Write out (or update) the information to an SQL db.

To do this, I felt that creating a hash that included all the test
results for each machine would be the simplest since I can use SQL
column headers for $Keys, I can then easily create SQL statements from
the HASH. However, the problem seems to be that I can't pass the HASH
that was created to a subroutine
because it wasn't defined when USE STRICT is ON, it works fine without
the parma, but I'd rather do it 'right'. As was pointed out, if I
pass a HASH by value, it will be flattened to a list, which is not
what I'm looking for. I'm open to suggestions on a different
approach.

Both Richard Gration and A. Sinan Unur suggested that perhaps I was
trying to pass %MachineInfo to &EcritureSQL. This was not the case
originally, &MachineInfo was meant to contains the existing (if any)
information from the database. I realized, after a bit of thought,
that there really isn't a reason to create a second HASH, since the
first one already contains all of the column headers as $Keys, all I
need to do is update the existing one with the new results and write
that to the SQL db. And that's one point for simplification.

I've modified the code slightly to reflect your suggestions (properly
scoped variables thru my, and proper formating for easy copy/paste
execution). But now the error message has moved to the SUB level, so
I guess that the HASH is being passed on correctly, except I can't
access the elements in it, I get the same message as before, "Can't
use string ("0") as a HASH ref while "strict refs" in use at...", line
is noted in the code below.

Thanks again for your time and insight.


Patrick Paquet

################################################################

use strict;
use Win32::ODBC;
use File::Find;
use Win32API::Net;
use Win32::NetAdmin;
use Net:ing;
use Net::hostent;
use Socket;

my @ListDOM = qw/Machine1 Machine2 Machine3/;

foreach my $Machine (@ListDOM)
{
my $CheckMachine = 0;
my %MachineInfo;
# my %MachineInfo = &ReadSQL($ConnectionSQL,$Machine);
# Previous line commented to allow execution, otherwise runs OK
if ($MachineInfo{MachineName} ne $Machine)
{
$CheckMachine = 1;
print "$Machine n'existe pas dans la base.($CheckMachine)\n";
}
my $ValeurCle = 123456;
# Previous line normally gets data from: &GetValeurCle($Machine)
if ($MachineInfo{Valeur_CiPatches} != $ValeurCle)
{
$CheckMachine = 1;
print "$Machine: valeur cle differente.($CheckMachine)\n";
}
my ($Ping,$ReverseLookup,$AdresseIP) = ("OK","OK","10.10.10.10");
# previous line normally gets data from: &GetIP($Machine)
if ($MachineInfo{IP_address} ne $AdresseIP)
{
$CheckMachine = 1;
print "$Machine: adresse IP differente.($CheckMachine)\n";
}
if ($CheckMachine == 0)
{
print "Base a jour pour $Machine, machine suivante...\n\n";
}
else
{
print "Checks values on workstation...\n\n";
EcritureSQL(%MachineInfo);
# previous line passes HASH to subroutine to write to SQL db
} # Fin de la verification d'une machine
}

sub EcritureSQL # Ecriture des infos dans la base SQL
{
# Doesn't do much at this point, until I get that error sorted out.
my ($Key, $Value, %InfosMachine);
my $InfosMachine = @_;

### ERROR IS ON FOLLOWING LINE
while (($Key, $Value)=each %$InfosMachine)

{
print "Key: $Key \->Value: $Value\n";
}
my $Pause = <STDIN>;
}

 

[Anno Siegel]

[...]

information from the database. I realized, after a bit of thought,
that there really isn't a reason to create a second HASH, since the
first one already contains all of the column headers as $Keys, all I
need to do is update the existing one with the new results and write
that to the SQL db. And that's one point for simplification.

Okay, but you ought to have some means to tell new information (that
isn't in the DB yet) from old information. Otherwise you'd have to
write back everything on every update.

I've modified the code slightly to reflect your suggestions (properly
scoped variables thru my, and proper formating for easy copy/paste
execution). But now the error message has moved to the SUB level, so

Yes. Let's concentrate on that.

[snip]

So you have a subroutine:

sub EcritureSQL # Ecriture des infos dans la base SQL
{
# Doesn't do much at this point, until I get that error sorted out.
my ($Key, $Value, %InfosMachine);

Declare your variables in the smallest possible scope. Declaring them
together with the first use (if possible) guarantees that. Scratch
the last line.

my $InfosMachine = @_;

Note that at this point you have declared two variables with the name
"InfosMachine", one hash (%InfosMachine) and one scalar ($InfosMachine).
These variables have nothing to do with each other, and setting one
will leave the other alone.

Here you are setting (the scalar) $InfosMachine to the array value of
@_. An array in scalar context is the number of its elements, so you
have just set $InfosMachine to the number of arguments your sub was
called with. Not useful.

Change the last line to

my %InfosMachine = @_;

### ERROR IS ON FOLLOWING LINE
while (($Key, $Value)=each %$InfosMachine)

Now you need

while ( my ($Key, $Value) = each %$InfosMachine )

{
print "Key: $Key \->Value: $Value\n";
}
my $Pause = <STDIN>;
}

EcritureSQL(%MachineInfo);

Now this call should work.

When you have this up and running you may want to change the passing of
the hash to a reference (which may win a little speed), but I suggest
you try the straight-forward way first.

Let me add that you have done a good job with the code you prepared.
Even though I didn't actually run it, it's a far easier read when you
can see the author made an effort to make it self-contained.

Anno

 

[A. Sinan Unur]

(e-mail address removed) (Patrick Paquet) wrote in

I'm replying to the original message because both Richard Gration and
A. Sinan Unur have provided interesting and useful information, I hope
this isn't a major breach of group etiquette.

I don't think so. Even if it were, you have done enough work to
compensate for that

....

I'm open to suggestions on a different approach.

I would prefer to use references to hashes all around.

Both Richard Gration and A. Sinan Unur suggested that perhaps I was
trying to pass %MachineInfo to &EcritureSQL. This was not the case
originally, &MachineInfo was meant to contains the existing (if any)

May I suggest dropping the & unless you really need it. (from perldoc
perlsub:

&NAME(LIST); # Circumvent prototypes.
&NAME; # Makes current @_ visible to called subroutine.

....

I've modified the code slightly to reflect your suggestions (properly
scoped variables thru my, and proper formating for easy copy/paste
execution).

You have not moved all your variables to the smallest possible scope;
there is still some room for improvment.

Thanks again for your time and insight.

You are welcome. I am sorry I do not have the time to go through
everything. I am including below a modified version of your script. I did
get rid of the comments because they were distracting to me.

I defined a placeholder subroutine called ReadSQL which returns a
reference to a hash holding the machine information given the machine's
name. After performing the checks, this reference is then passed on to
EcritureSQL. Notice the scoping of $Key and $Value in that sub as well as
the

my $InfosMachine = shift;

line as the first line of the sub. That makes it immediately obvious to
the reader what the parameters to the sub are. (Compare that to the first
line of the sub in your version ... which of those variables are
parameters, which ones are local to the subroutine?)


$ cat t.pl
use strict;
use warnings;
use diagnostics;

my @ListDOM = qw/Machine1 Machine2 Machine3/;

foreach my $Machine (@ListDOM) {
my $CheckMachine = 0;
my $MachineInfo = ReadSQL('dummy', $Machine);
if ($MachineInfo->{MachineName} ne $Machine) {
$CheckMachine = 1;
print "$Machine n'existe pas dans la base.($CheckMachine)\n";
}
my $ValeurCle = 123456;
if ($MachineInfo->{Valeur_CiPatches} != $ValeurCle) {
$CheckMachine = 1;
print "$Machine: valeur cle differente.($CheckMachine)\n";
}
my ($Ping, $ReverseLookup, $AdresseIP) = ('OK', 'OK', '10.10.10.10');

if ($MachineInfo->{IP_address} ne $AdresseIP) {
$CheckMachine = 1;
print "$Machine: adresse IP differente.($CheckMachine)\n";
}

if ($CheckMachine == 0) {
print "Base a jour pour $Machine, machine suivante...\n\n";
} else {
print "Checks values on workstation...\n\n";
EcritureSQL($MachineInfo);
}
}

sub EcritureSQL {
my $InfosMachine = shift;

while (my ($Key, $Value) = each %$InfosMachine) {
print "Key: $Key \->Value: $Value\n";
}
my $Pause = <STDIN>;
}

sub ReadSQL {
my ($ConnectionSQL, $Machine) = @_;
return {
MachineName => $Machine,
Valeur_CiPatches => 0,
IP_address => '10.10.10.10',
};
}
__END__

$ perl t.pl
Machine1: valeur cle differente.(1)
Checks values on workstation...

Key: Valeur_CiPatches ->Value: 0
Key: IP_address ->Value: 10.10.10.10
Key: MachineName ->Value: Machine1

Machine2: valeur cle differente.(1)
Checks values on workstation...

Key: Valeur_CiPatches ->Value: 0
Key: IP_address ->Value: 10.10.10.10
Key: MachineName ->Value: Machine2

Machine3: valeur cle differente.(1)
Checks values on workstation...

Key: Valeur_CiPatches ->Value: 0
Key: IP_address ->Value: 10.10.10.10
Key: MachineName ->Value: Machine3

 

[Patrick Paquet]

Hello,

Again responding to my previous post, as both Anno Siegel and A. Sinan
Unur provided useful methods to fix my problem. I tested both out on
my code and both worked.

Okay, but you ought to have some means to tell new information (that
isn't in the DB yet) from old information. Otherwise you'd have to
write back everything on every update.

True. I've been thinking about that as well, for the moment I'm
concentrating on checking a few 'key' values, if any of those have
changed then I update the lot. Not the most efficient, but if I find
it to be too lengthy I'll look at cleaning that up. Since this is a
replacement for an existing script (that is WAY heavier...), just
skipping over the workstations that haven't changed is a major
performance boost.

Declare your variables in the smallest possible scope. Declaring them
together with the first use (if possible) guarantees that. Scratch
the last line.

Damn... Thought I had that sorted out. I seem to have a tenious
grasp on scope at this point.

When you have this up and running you may want to change the passing of
the hash to a reference (which may win a little speed), but I suggest
you try the straight-forward way first.

This is another thing I have a tenious (at best!) grasp of, hashes,
and references to hashes. I find it all very confusing, and usually
end up trying out a bunch of syntax permutations until I find
something that works. But I'm getting better at that too.

Let me add that you have done a good job with the code you prepared.
Even though I didn't actually run it, it's a far easier read when you
can see the author made an effort to make it self-contained.

Thanks, I read the posting guidelines... Also noticed that
properly written out posts got useful replies... And the whole
process of making the thing self contained, and making sure that the
code would run when copy/pasted was educationnal as well.


And...

(e-mail address removed) (Patrick Paquet) wrote in
news:[email protected]:

I would prefer to use references to hashes all around.

Again with the references to hashes, gonna have to wrap my head around
this.

May I suggest dropping the & unless you really need it. (from perldoc
perlsub:

Done, works fine without &.

You have not moved all your variables to the smallest possible scope;
there is still some room for improvment.

<Sigh> Scope... I'll get that right too, at some point.

my $InfosMachine = shift;

That's interesting, had to look up SHIFT, and think it thru for a few
minutes. So Shift passes the first element (the hash reference) to
$InfosMachine. And you're right, it makes things pretty clear. If
you have time, I'd be curious to see how you would handle multiple
arguments to a sub, for example (off the top of my head.... really):

EcriteSQL($SQLConnect,$RecordStatus,\%InfoMachine)

I guess shift is not useable in that context since it only returns the
first element from the array.(?)

And after all that, I've got the script running properly now, and I
also have a better (although not yet perfect) grasp of scope, hashes
(and their references) and clean code. I also learned that taking the
time to properly formulate a question will most likely get me a proper
answer (the old stupid question/stupid answer thing I guess).

Thanks to all of you for taking the time to help me understand all
this. Hope everybody has a good day.


Patrick Paquet

 

[Anno Siegel]

Hello,

Again responding to my previous post, as both Anno Siegel and A. Sinan
Unur provided useful methods to fix my problem. I tested both out on
my code and both worked.

(e-mail address removed)-berlin.de (Anno Siegel) wrote in message


True. I've been thinking about that as well, for the moment I'm
concentrating on checking a few 'key' values, if any of those have
changed then I update the lot. Not the most efficient, but if I find
it to be too lengthy I'll look at cleaning that up. Since this is a
replacement for an existing script (that is WAY heavier...), just
skipping over the workstations that haven't changed is a major
performance boost.

Okay, let's leave that aside for the moment. (You can, of course,
use a second hash to store only the new keys, and write only them back
if necessary.)

Damn... Thought I had that sorted out. I seem to have a tenious
grasp on scope at this point.

I hope you didn't overlook the more severe error of assigning an array
to a scalar. That's what made your attempt fail.

[...]

Thanks, I read the posting guidelines... Also noticed that

Good thing

properly written out posts got useful replies... And the whole
process of making the thing self contained, and making sure that the
code would run when copy/pasted was educationnal as well.

That last point can't be stressed enough. Explaining a problem clearly
helps understand it better, sometimes to the point of finding a solution.

Anno

 

[Tad McClellan]

That's interesting, had to look up SHIFT,

Case matters. Perl has a shift(), it does not have a SHIFT().

and think it thru for a few
minutes. So Shift passes the first element (the hash reference) to
$InfosMachine.

AND it removes that first element from @_, making it one element shorter.

And you're right, it makes things pretty clear. If
you have time, I'd be curious to see how you would handle multiple
arguments to a sub, for example (off the top of my head.... really):

EcriteSQL($SQLConnect,$RecordStatus,\%InfoMachine)

I guess shift is not useable in that context since it only returns the
first element from the array.(?)

You can always call shift() another time:

my $connect = shift; # get 1st argument
my $status = shift; # get 2nd argument
my $href = shift; # @_ now has 3 less elements in it

or, better IMO:

my($connect, $status, $href) = @_; # @_ still has all original elements

And after all that, I've got the script running properly now, and I
also have a better (although not yet perfect) grasp of scope, hashes

Both variable scope and hashing are taught in the usual CS curriculum,
you'll need to pick them up yourself if you are a self-taught programmer.

I can't remember if this article has been mentioned in this thread yet:

"Coping with Scoping":

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

 

[A. Sinan Unur]

(e-mail address removed) (Patrick Paquet) wrote in

That's interesting, had to look up SHIFT, and think it thru for a few
minutes. So Shift passes the first element (the hash reference) to
$InfosMachine. And you're right, it makes things pretty clear. If
you have time, I'd be curious to see how you would handle multiple
arguments to a sub, for example (off the top of my head.... really):

EcriteSQL($SQLConnect,$RecordStatus,\%InfoMachine)

I guess shift is not useable in that context since it only returns the
first element from the array.(?)

Well, one could do:

EcriteSQL($SQLConnect,$RecordStatus,\%InfoMachine) {
my $SQLConnect = shift;
my $RecordStatus = shift;
my $InfoMachine =shift;
}

But, that is cumbersome. I did not mean to stress the use of shift, but
rather, the practice of initializing your subroutine's lexical variables
from parameters as the first thing to do in the sub. In fact there has been
some discussion in this group about why one should stick with

sub somesub {
my $var = @_;
}

rather than

sub somesub {
my $var = shift;
}

unless one really wants to remove elements from @_.

 

[Patrick Paquet]

I hope you didn't overlook the more severe error of assigning an array
to a scalar. That's what made your attempt fail.

Indeed, that was sorted out. I ended up using A. Sinan Unur's method
of passing along the reference to the HASH. ie:

my $InfosMachine = shift;

in the sub.

Actually, I now use that method for all the subs where I have only one
argument to pass.

Anno

Thanks for your help!


Patrick