in reply to OO package problems...

The other answers have solved your immediate problem. I would like to address a style issue.

When working with Object Orientation, there are a number of considerations you should take into account in order to get the most out of the OO paradigm. I've noticed that nearly every "OO" implementation does not follow at least one of these.

  1. Encapsulation. You do this by putting the actual data values into their own namespace. This is good. However, you allow anyone to just come in and muck with the data. This is legal OO ... just not good OO.
  2. Inheritance. While you don't show us much of the full class hierarchy, I'm going to assume that you do have a system of classes and use @ISA and the like. However, I immediately note that you don't do things like my $self = SUPER::new(@_);, allowing for a generic constructor. If you ever wanted to change how the objects were implemented from a hash to, say, a closure or parallel lists, you would have to completely rewrite every single class.
  3. Polymorphism. This is where you allow for classes to use their parents' methods. In fact, this is where classes are supposed to use their parents' methods. They are only supposed to override them, or add new methods, if they need to. This is where you get the benefits of code reuse.
What you should be looking at doing is giving the object (in this case, some sort of magical cloak) its attribute values, then having the object answer questions about itself. Not simple questions like "What is your arrayref of stat modifiers?", but questions like "What is the modification you give for XXX?". This allows for you to change how that is implemented internally, yet not change every piece of code that would use that method.

Below, I suggest a possible rewrite of this class. This rewrite is dependent on there being a base class of all objects, then a base class of all magical objects. In addition, there should be a class for each of the stats, which would contain things like "How much can this much strength lift?" or "What is the armor modifier for this much Dex?" and the like.

use 5.6.0; use strict; use warnings; package Item; my @OPTIONS = qw(ATYPE DESC FILLABLE MASS NAME RACE VOL VAL ); use constant STR => (0); use constant DEX => (1); sub new { my $class = shift; # Create a separate copy constructor. return undef if defined $class; my $self = {} bless $self, $class; my %args = @_; foreach my $option (@OPTIONS) { $self->{$option} = $args{$option} || undef; } $self->{STATS_CHANGE} = $args{STATS_CHANGE} || [0,0]; return $self; } sub name { $_[1] ? $_[0]->{NAME} = $_[1] : $_[0]->{NAME} }sub vol { $ +_[1] ? $_[0]->{VOL} = $_[1] : $_[0]->{VOL} } sub val { $_[1] ? $_[0]->{VAL} = $_[1] : $_[0]->{VAL} } sub desc { $_[1] ? $_[0]->{DESC} = $_[1] : $_[0]->{DESC} } sub fillable { $_[0]->{FILLABLE} } sub mass { $_[0]->{MASS} } sub atype { $_[0]->{ATYPE} } sub race { $_[0]->{RACE} } sub stats_change { $_[0]->{STATS_CHANGE}[$_[1]] } 1; __END__
Note that some of the attributes are now read-only. This is because, I'm assuming, that Armor Type and Race-Allowed shouldn't be changing for the life of an item. Also, FILLABLE and MASS need to be more complex calculations, based on whether or not there's stuff inside this item. Both should be read-only, changing only due to internal considerations, such as how much stuff is in this item.

stats_change() now takes a attribute (which is defined as a constant), and returns what the stat change is for that specific attribute. In addition, it's read-only. Your previous way was forcing the caller to know about internal implementation. This prevents you from easily changing the implementation of this object.

Quick note - the use of constants to interface an array is a poor choice, but better than what you were doing. It is better instead to use objects for attribute types and have the item use a Stats::Modifier object which interacts with a Stats object. It sounds like a lot more work, and it is, but it makes your life a lot easier down the road.

------
We are the carpenters and bricklayers of the Information Age.

Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.

Replies are listed 'Best First'.
Re: Re: OO package problems...
by Fletch (Bishop) on Oct 23, 2001 at 20:50 UTC

    Also check out Class::MethodMaker which will automate generating much of this code for you. You just give it a list of attributes and what not and it writes all of the mostly boilerplate code for accessors. You can still write your own custom accessors for things that C::MM can't handle itself.

    And consider getting the excellent book, Object Oriented Perl.