On Tue, Sep 04, 2001 at 06:19:34PM -0400, David Ferrance wrote:
> BTW a lot of you know perl better than I do. I love constructive criticism. Fire
>away.
Well, since you ask, here are a few things that jumped out at me during a
quick read of the code.
# load in options supplied to new()
for (my $x = 0; $x <= $#_; $x += 2)
$x <= $#_ is wrong. $#_ contains the index of the last item. Since you are
stepping by twos, $x < $#_ is what you want.
$options->{lc($_[$x])} = $_[($x + 1)];
Where is $options defined?
bless($self);
This should either use pass the C<$class> variable, or get rid of it and the
and the C<$proto> variable. As it stands, you set it's value, but do not use it
anywhere else.
I usually say C<$self = bless {}, ref($self) || $self> somewhere near the top of the
sub.
As written, this package cannot be easily subclassed.
return $self;
}
sub DESTROY
{
my $self = shift;
eh what? Are you trying to keep this thing in memory forever?
If you run this under mod_perl, you'll only reclaim the memory when you restart the
httpd server.
}
sub define_total
{
my ($self,$col) = @_;
# should verify that col hasn't already been added
You could keep a hash of column names to do the verification. Question is,
what do you do in the event of a duplicate? Die or replace original column with new
value I suppose.
# OK, add it
push @{$self->{SUM_COLS}}, $col;
}
-Gyepi
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
