Storing object references in hashes

[Bryan Balfour]

I'm loathe to post this as I'm sure the problem is my misunderstanding
of how perl handles references but I've been going round in circles
long enough to justify my plea for help.

I've reached the stage in learning perl where I want to use OOP
techniques and I seem to have fallen at the first hurdle. Basically I
wanted to create multiple instances of a class and store the addresses
of these instances in a hash. This hash is then sorted on key for use
later on in the processing to reference the data elements stored in
each instance.
The problem I'm getting is that, although the hash seem to contain the
correct references to the object, using these references to address the
relevant object always results in the information from the last object
created being returned.

The code below demonstrates the basics of what I'm attempting. Running
it at a DOS prompt produces the following output.......

Unsorted list...........
Name: Bryan, Age: 63, Object: Person=HASH(0x1847ac0)
Name: Anne, Age: 62, Object: Person=HASH(0x1882fc0)
Name: Kevin, Age: 39, Object: Person=HASH(0x1847afc)
Name: Gareth, Age: 35, Object: Person=HASH(0x1847af0)
Name: Siobhan, Age: 31, Object: Person=HASH(0x1847b50)
Name: Kathryn, Age: 28, Object: Person=HASH(0x1847b8c)
Sorted list...........
Hash{name: Anne, object: Person=HASH(0x1882fc0)} {Object{name:
Kathryn, Age: 28}
Hash{name: Bryan, object: Person=HASH(0x1847ac0)} {Object{name:
Kathryn, Age: 28}
Hash{name: Gareth, object: Person=HASH(0x1847af0)} {Object{name:
Kathryn, Age: 28}
Hash{name: Kathryn, object: Person=HASH(0x1847b8c)} {Object{name:
Kathryn, Age:28}
Hash{name: Kevin, object: Person=HASH(0x1847afc)} {Object{name:
Kathryn, Age: 28}
Hash{name: Siobhan, object: Person=HASH(0x1847b50)} {Object{name:
Kathryn, Age:28}

* * * * * * * * * * *
I'd really appreciate someone pointing out were I'm going wrong here as
it has got me stumped.

Regards,

Bryan Balfour
* * * * * * * * * * *
use warnings;
use strict;
use constant TRUE => 1;
use constant FALSE => 0;
use Tie::IxHash;
my(%people, %persons, $person, $name, $age, $object);
tie %people, "Tie::IxHash"; # Maintain loading order
%people = ("Bryan" , 63, "Anne" , 62, "Kevin", 39, "Gareth", 35,
"Siobhan", 31, "Kathryn", 28);
print "Unsorted list...........\n";
while(($name, $age) = each(%people))
{
$person = Person->new($name, $age); # Get new instance of Person
class and load name and age
$persons{$name} = $person; # New hash entry with name as
key and and new object as value
print "Name: " . $person->getName() . ", Age: " . $person->getAge() .
", Object: " . $person . "\n";
}
print "Sorted list...........\n";
foreach $name (sort(keys(%persons)))
{
$person = $persons{$name}; # Person object built for this
name
print "Hash{name: " . $name . ", object: " . $person . "}
{Object{name: " . $person->getName() .
", Age: " . $person->getAge() . "}\n";
}
exit(TRUE);

package Person;

use constant TRUE => 1;
use constant FALSE => 0;
my($thisName, $thisAge);

sub new
{
my($className) = shift(@_);
($thisName, $thisAge) = @_;
my($self) = {};
bless($self, $className);
return($self);
}

sub getName
{
my($self) = shift(@_);
return($thisName);
}

sub getAge
{
my($self) = shift(@_);
return($thisAge);
}
return(TRUE);

sub getAge
{
my($self) = shift(@_);
return($thisAge);
}
return(TRUE);

 

[Uri Guttman]

BB> my($thisName, $thisAge);

why are those declares as file lexicals? they are global to all your objects.

BB> sub new
BB> {
BB> my($className) = shift(@_);
BB> ($thisName, $thisAge) = @_;

you don't do anything with those. they work when you initialize the
Person object since they know the just created person. but you don't
store them in the object itself

BB> my($self) = {};

your object there is empty. no way could you print out anything useful
from it.

BB> bless($self, $className);
BB> return($self);
BB> }

BB> sub getName
BB> {
BB> my($self) = shift(@_);
BB> return($thisName);

and how would $thisName be anything but the last name used in new()? it
needs to be part of the object.


you seem to have a major misconception on how perl hash based objects
are created. their members are not declared as file lexical but as
members of the hash object.

get the book 'object oriented perl' and you will learn all about how to
do this. also you could read the various perl docs on the subject:

perlboot Perl OO tutorial for beginners
perltoot Perl OO tutorial, part 1
perltooc Perl OO tutorial, part 2
perlobj Perl objects

uri

 

[Paul Lalli]

The problem I'm getting is that, although the hash seem to contain the
correct references to the object, using these references to address the
relevant object always results in the information from the last object
created being returned.

Correct. Which means it's a problem with your object creation, not
with the storing in the hash or the sorting.

I'd really appreciate someone pointing out were I'm going wrong here as
it has got me stumped.

Your central problem is scoping. As a general rule, you should always
declare your variables in the smallest scope possible. This is the
direct cause of your error.

use warnings;
use strict;
GOOD!!

use constant TRUE => 1;
use constant FALSE => 0;

why? Why not just use 1 and 0? I'm guessing you're a C programmer
trying to make his way in Perl, right? Program Perl. Do not program C
in Perl.

use Tie::IxHash;
my(%people, %persons, $person, $name, $age, $object);

This what I'm talking about. The only variable that needs to be
declared in this scope is %people. All the rest should be declared
later.

(Oh, and I'm very nervous about having two variables so similarly named
as %people and %persons)

tie %people, "Tie::IxHash"; # Maintain loading order
%people = ("Bryan" , 63, "Anne" , 62, "Kevin", 39, "Gareth", 35,
"Siobhan", 31, "Kathryn", 28);

Try to use the => notation when creating a hash. This is a standard
idiom, it increases readability, and it lets you not have to quote each
of those keys.

%people = (Bryan => 63, Anne => 62, Kevin => 39, Gareth => 35, Siobhan
=> 31, Kathryn => 28);

print "Unsorted list...........\n";
while(($name, $age) = each(%people))

Here is where $name and $age should be declared:

while (my ($name, $age) = each %people) {

{
$person = Person->new($name, $age); # Get new instance of Person
class and load name and age

Here is where $person should be declared:
my $person = Person->new($name, $age);

$persons{$name} = $person; # New hash entry with name as
key and and new object as value
print "Name: " . $person->getName() . ", Age: " . $person->getAge() .
", Object: " . $person . "\n";
}
print "Sorted list...........\n";
foreach $name (sort(keys(%persons)))

foreach my $name (sort keys %persons) {

{
$person = $persons{$name}; # Person object built for this
name
print "Hash{name: " . $name . ", object: " . $person . "}
{Object{name: " . $person->getName() .
", Age: " . $person->getAge() . "}\n";
}
exit(TRUE);

package Person;

use constant TRUE => 1;
use constant FALSE => 0;
my($thisName, $thisAge);

And here's our problem. Because these two variables are declared at
the package-scope level, each time new() is called, new() is just
reusing those two variables. Changing them to be whatever was passed
in to new() each time.

sub new
{
my($className) = shift(@_);
($thisName, $thisAge) = @_;

my ($thisName, $thisAge) = @_;

my($self) = {};

You have a severe lack of understanding of modules. There is no reason
to declare a hash reference if you're not going to store the class
attributes in it:

my $self = { name => $thisName, age => $thisAge };

bless($self, $className);
return($self);
}

sub getName
{
my($self) = shift(@_);
return($thisName);

Didn't this strike you as odd? How was Perl to know that $thisName it
was returning was at all associated with $self?

return $self->{name};

}

sub getAge
{
my($self) = shift(@_);
return($thisAge);

return $self->{age};

}
return(TRUE);

Paul Lalli

 

[Paul Lalli]

get the book 'object oriented perl' and you will learn all about how to
do this.

With all due respect to Mr. Conway, allow me to recommend as an
alternative "Learning Perl Objects, References, and Modules", by Randal
Schwartz. This is especially true if the OP has read "Learning Perl"
and liked the style of it, as the "Alpaca" is a direct sequel to the
"Llama".

Paul Lalli

 

[Ch Lamprecht]

* * * * * * * * * * *

I'd really appreciate someone pointing out were I'm going wrong here as
it has got me stumped.

Regards,

Bryan Balfour
* * * * * * * * * * *
use warnings;
use strict;
use constant TRUE => 1;
use constant FALSE => 0;
use Tie::IxHash;
my(%people, %persons, $person, $name, $age, $object);
tie %people, "Tie::IxHash"; # Maintain loading order
%people = ("Bryan" , 63, "Anne" , 62, "Kevin", 39, "Gareth", 35,
"Siobhan", 31, "Kathryn", 28);
print "Unsorted list...........\n";
while(($name, $age) = each(%people))
{
$person = Person->new($name, $age); # Get new instance of Person
class and load name and age
$persons{$name} = $person; # New hash entry with name as
key and and new object as value
print "Name: " . $person->getName() . ", Age: " . $person->getAge() .
", Object: " . $person . "\n";
}
print "Sorted list...........\n";
foreach $name (sort(keys(%persons)))
{
$person = $persons{$name}; # Person object built for this
name
print "Hash{name: " . $name . ", object: " . $person . "}
{Object{name: " . $person->getName() .
", Age: " . $person->getAge() . "}\n";
}
exit(TRUE);

Hi Bryan,

package Person;

use constant TRUE => 1;
use constant FALSE => 0;
my($thisName, $thisAge);

-------^ -------^
Here you declare lexical variables...

sub new
{
my($className) = shift(@_);

and here you assign to them...
instead of storing the instance data in your object-ref:

($thisName, $thisAge) = @_;
my($self) = {};

could be :

my($self) = {name =>$_[0],
age =>$_[1]};

bless($self, $className);
return $self;
}

The same with your getters: Return values from the object-ref

sub getName
{
my$self = shift;
return $self->{name};
}

sub getAge
{
my$self = shift;
return $self->{age};
}

hth Christoph

 

[Bryan Balfour]

Hi Paul,

Thanks for that. I'll take a look at both books.

Bryan

 

[Bryan Balfour]

Hi Christoph,

Many thanks for getting back.

When I saw your re-coding of my original it became so obvious what I
was doing wrong. My perception of variables declared in the main
section of the Package (Class) was that they were 'object level'
variables whereas they are in fact 'class level' variables. Once I'd
realised this my problem was obvious. Having implemented your
suggestions, the codes works fine.

Viel Dank,

Bryan

 

[Bryan Balfour]

Hi Paul,

Many thanks for getting back.

Thanks to your's and other's posts I now realise that my understanding
of scoping at the Package level was wrong. I'd assumed that variables
declared in the main section are 'object level' variables whereas they
are in fact 'class level' variables. Once I'd realised this my problem
was obvious. I think you can see that, with this misconception, I
didn't think the code looked odd

On your others points;

No, I'm not a C programmer. It's much worse that that. For many years I
wrote applications for IBM mainframes using Basic Assembler Language
and the like so I'm well conversant with modules. My problem was
getting into a mind set which was flawed. I'd got into this mind set
because, up to now, I've only used the perl Package capability to break
my code up into discrete logical sections. Because I instantiated each
package only once in an application, it worked. I now realise that
there was no need to instantiate them at all. All I had to do was
reference their methods at the class level.

TRUE/FALSE v's 1/0. Using TRUE/FALSE means more to me than 1/0 so,
until it's demonstarted to me that it's poor programming, I'll
continue. I've used this technique for years, together with ON/OFF etc,
and I'm too old to change now without a good reason. Like the way I lay
out my code, it's a personal preference that suits me and we should all
be allowed those so long as they work.

Use the '=>' notation. Good point. I'll do this in future. It's much
more readable.

Choice of variable names (%people and %persons). Agreed, I'd not
usually do this but it was just a piece of code I'd put together to
demonstrate my problem.

Variable scoping at the lowest possible level. Agreed, but I had a few
problems when I tried to do this as you suggested. The best I could do
is shown in the listing of the current code below. I'd appreciate it if
you could point out any further improvements. (As way of celebration,
I've extended the code to list the names sorted on age.)

Many thanks for your help,

Bryan

* * * * * * * *
* * * * * * * *
use warnings;
use strict;
use constant TRUE => 1;
use constant FALSE => 0;
use Tie::IxHash;
my(%people, %persons);
tie %people, "Tie::IxHash"; # Maintain loading
order
%people = (Bryan => 63, Anne => 62, Kevin => 39, Gareth => 35, Siobhan
=> 31, Kathryn => 28);
print "Unsorted list...........\n";
my($person);
while(my($peopleName, $peopleAge) = each(%people))
{
$person = Person->new($peopleName, $peopleAge); # Get new instance
of Person class and load name and age
$persons{$peopleName} = $person; # New hash
entry with name as key and and new object as value
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
.. "\n";
}
print "List sorted on name...........\n";
my($personsName);
foreach $personsName (sort(keys(%persons)))
{
$person = $persons{$personsName}; # Person object
built for this name
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
.. "\n";
}
print "List sorted on age...........\n";
foreach $personsName (sort {$people{$a} cmp $people{$b}} keys %people)
{
$person = $persons{$personsName}; # Person object
built for this name
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
.. "\n";
}
exit(TRUE);

package Person;

use constant TRUE => 1;
use constant FALSE => 0;

sub new
{
my($className) = shift(@_);
my($self) = {name =>$_[0],
age =>$_[1]};
bless($self, $className);
return($self);
}

sub getName
{
my($self) = shift(@_);
return $self->{name};
}

sub getAge
{
my($self) = shift(@_);
return $self->{age};
}
return(TRUE);

 

[Paul Lalli]

Many thanks for getting back.

You're welcome. However, I must ask that you start quoting some
context when posting a reply. Not everyone can or chooses to view
Usenet in a threaded manner, and there is no guarantee that every post
will reach a given newserver in the "right" order, or even at all.

Thanks to your's and other's posts I now realise that my understanding
of scoping at the Package level was wrong. I'd assumed that variables
declared in the main section are 'object level' variables whereas they
are in fact 'class level' variables. Once I'd realised this my problem
was obvious. I think you can see that, with this misconception, I
didn't think the code looked odd

It's been a while since I did any OOP in C++ or similar languages, so
looking back, I guess I can see where you're coming from.

TRUE/FALSE v's 1/0. Using TRUE/FALSE means more to me than 1/0 so,
until it's demonstarted to me that it's poor programming, I'll
continue. I've used this technique for years, together with ON/OFF etc,
and I'm too old to change now without a good reason.

My only issue with it is that it requires more typing (both by way of
typing TRUE instead of 1, and the declarations of the constants in
every package), for seemingly little benefit.

Variable scoping at the lowest possible level. Agreed, but I had a few
problems when I tried to do this as you suggested.

Could you elaborate on what those problems were?

The best I could do
is shown in the listing of the current code below. I'd appreciate it if
you could point out any further improvements. (As way of celebration,
I've extended the code to list the names sorted on age.)

use warnings;
use strict;
use constant TRUE => 1;
use constant FALSE => 0;
use Tie::IxHash;
my(%people, %persons);
tie %people, "Tie::IxHash"; # Maintain loading order
%people = (Bryan => 63, Anne => 62, Kevin => 39, Gareth => 35, Siobhan => 31, Kathryn => 28);
print "Unsorted list...........\n";
my($person);

Okay. You're declaring one lexical named $person, but using it in two
completely un-related blocks. That is, the last value of $person in
the first block will carry over to the value of $person in the second
block. It is much better to declare two lexicals, one for each block.
(Well, I don't know about "much" better, but certainly "better", IMO)

while(my($peopleName, $peopleAge) = each(%people))
{
$person = Person->new($peopleName, $peopleAge);

my $person = Person->new($peopleName, $peopleAge);

# Get new instance of Person class and load name and age
$persons{$peopleName} = $person;
# New hash entry with name as key and and new object as value
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge() . "\n";
}
print "List sorted on name...........\n";
my($personsName);

Same here. Lexically scope $personsName each time you need it, unless
you want the values to persist between blocks. (Here, you don't)

foreach $personsName (sort(keys(%persons)))

for my $personsName (sort keys %persons)

{
$person = $persons{$personsName};

my $person = $persons{$personsName};

# Person object built for this name
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
. "\n";
}
print "List sorted on age...........\n";
foreach $personsName (sort {$people{$a} cmp $people{$b}} keys %people)

1) lexically declare a new $personsName
2) This sort works only because all your ages happen to be two digits.
Try adding a person of age 5 or age 101, and see what results you get.
You need to tell the sort to compare numerically, not ASCIIbetically:

{
$person = $persons{$personsName};

my $person = $persons{$personsName};

# Person object built for this name
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
. "\n";
}
exit(TRUE);

package Person;

Remember that if you eventually move the Person package into its own
module (Person.pm), you should enable strict and warnings here, too, as
they will not carry over from the scope of the "main" file.

use constant TRUE => 1;
use constant FALSE => 0;

sub new
{
my($className) = shift(@_);
my($self) = {name =>$_[0],
age =>$_[1]};
bless($self, $className);
return($self);
}

sub getName
{
my($self) = shift(@_);
return $self->{name};
}

sub getAge
{
my($self) = shift(@_);
return $self->{age};
}
return(TRUE);

My only other comment is that you seem to be parenthesis-happy.
Keep in mind that 90% of the time in Perl, parens are only needed if
their lack of existance would change precedence or context. Here, from
the line package Person; on down, not a single parentheses pair is
required.

Well, I could try to take issue with the BSD-style of indentation and
bracketing, as opposed to the far-superior K&R style, but I won't.

(No, really, I don't want this flame war! Please, stop! I'm sorry!!
Ahhhhhhh....)

Paul Lalli

 

[A. Sinan Unur]

Bryan Balfour wrote: ....


My only issue with it is that it requires more typing (both by way of
typing TRUE instead of 1, and the declarations of the constants in
every package), for seemingly little benefit.

"False" in Perl is not just zero. In particular, the empty string and
empty list are among the possible false values that can be returned.
"True" is not just 1, "0.0" is true as well.

While one may find it easy construct the correct returns/comparisons in
one's own code Perl itself, and modules do use the full range of values.

#define TRUE 1
#define FALSE 0

was never a good idea in C or C++, and it is not in Perl.

Sinan

 

[Bryan Balfour]

"True" is not just 1, "0.0" is true as well.

That may be a good enought reason for dropping the idea. Thanks for
pointing out.

Bryan Balfour

 

[Bryan Balfour]

You're welcome. However, I must ask that you start quoting some
context when posting a reply. Not everyone can or chooses to view
Usenet in a threaded manner, and there is no guarantee that every post
will reach a given newserver in the "right" order, or even at all.

I've been replying by clicking on the word 'Reply' at the bottom of the
post. This gives a blank screen. I now realise that I should have been
clicking on 'show options' at the top of the post then 'Reply' to get
the original text with the >s. I wondered were they came from. Now I
know.

My only issue with it is that it requires more typing (both by way of
typing TRUE instead of 1, and the declarations of the constants in
every package), for seemingly little benefit.

I'll take your's and others' comments on this onboard and may well drop
TRUE/FALSE.

Could you elaborate on what those problems were?

After some further playing following your post I've resolved them all.
With my passion for parentheses I'd coded:
foreach my($personsName) (sort(keys(%persons)))
and never thought to remove them.

Okay. You're declaring one lexical named $person, but using it in two
completely un-related blocks. That is, the last value of $person in
the first block will carry over to the value of $person in the second
block. It is much better to declare two lexicals, one for each block.
(Well, I don't know about "much" better, but certainly "better", IMO)

You're right. I'll do that in future.

2) This sort works only because all your ages happen to be two digits.
Try adding a person of age 5 or age 101, and see what results you get.
You need to tell the sort to compare numerically, not ASCIIbetically:

for my $personName (sort {$people{$a} <=> $people{$b}) keys %people)

Well spotted. Thanks for that.

My only other comment is that you seem to be parenthesis-happy.
Keep in mind that 90% of the time in Perl, parens are only needed if
their lack of existance would change precedence or context. Here, from
the line package Person; on down, not a single parentheses pair is
required.

This habit comes from the desire to removing ambiguity. Not sure though
if I can just quit this addiction but I'll try and cut down gradually.
As I'm used to using parentheses, the code looks odd when I remove them
but I guess I'll get used to it.

Well, I could try to take issue with the BSD-style of indentation and
bracketing, as opposed to the far-superior K&R style, but I won't.

I never realised my style had a name. According to Wikipedia, under
Indent Style, it's called Whitesmiths after the code examples used by a
company called Whitesmiths Ltd for their compiler released in 1977. As
I've been using this style since I started programming way back in
1969, I think this style should be renamed the Balfour style (if only
for 15 minutes).

You've been a great help. Paul. Many thanks.

Bryan Balfour

To help any others who may stumble upon this thread, I've included the
working code that, I hope, takes onboard all the recommendations of the
submitters (with one exception). Sorry, Paul but I'm sticking with the
Balfour style, for the moment anyway. No offence.

* * * * * * * * * * * * * *

use warnings;
use strict;

use Tie::IxHash;

my %people;
tie %people, "Tie::IxHash"; # Maintain loading order
%people = (Bryan => 63,
Anne => 62,
Kevin => 39,
Gareth => 35,
Elouise => 1,
Norman => 101,
Siobhan => 31,
Kathryn => 28);

print "Unsorted list...........\n";

my %persons;

while(my($peopleName, $peopleAge) = each %people)
{
# Get new instance of Person class and load name and age
my $person = Person->new($peopleName, $peopleAge);
# New hash entry with name as key and and new object as value
$persons{$peopleName} = $person;
print "Name: " . $person->getName() . ",\tAge: " . $person->getAge()
.. "\n";
}

print "List sorted on name...........\n";

foreach my $personsName (sort keys %persons)
{
# Retrieve Person object built for this name
my $person = $persons{$personsName};
print "Name: " . $person->getName() .
",\tAge: " . $person->getAge() . "\n";
}

print "List sorted on age...........\n";

foreach my $personsName (sort {$people{$a} <=> $people{$b}} keys
%people)
{
# Retrieve Person object built for this name
my $person = $persons{$personsName};
print "Name: " . $person->getName() .
",\tAge: " . $person->getAge() . "\n";
}
exit 0;

package Person;

use warnings;
use strict;

sub new
{
my $className = shift @_;
my $self = {name => $_[0],
age => $_[1]};
bless $self, $className;
return $self;
}

sub getName
{
my $self = shift @_;
return $self->{name};
}

sub getAge
{
my $self = shift @_;
return $self->{age};
}

return 1;