Re: Proper class setup? Mathew Snyder schreef:

> I'm presently learning OOP as Perl does it using online resources and
> and Programming Perl as my tutors.  I'm not certain I have it right
> though.  Is this correct for the package:

[whitespace is cheap]

> package Report;
>
> require Exporter;
> use strict;
>
> our @ISA       = qw(Exporter);
> our @EXPORT    = qw(new);

Exporting is not necessary, you can leave these two lines out.


> sub new {
>         my ($class) = @_;
>         my $self    = {
>                 _id               => undef,
>                 _queue    => undef,
>                 _owner    => undef,
>                 _priority => undef,
>                 _worked   => undef,
>                 _timeLeft => undef,
>                 _due      => undef,
>                 _created  => undef,
>                 _updated  => undef,
>                 _severity => undef,
>                 _ccl      => undef
>         };
>         bless $self, $class;
>         return $self;
> }

Change your new() in a new() and an init(). Do as little as possible in
your new(), for the sake of inheritance etc.

Although there is no real need to prepare the hash with undef values. So
"my $self = {};" suffices, and keeps your objects lean.


> # Accessor method for Reports _id
> sub id {
>         my ($self, $id) = @_;
>         $self->{_id} = $id if defined($id);
>         return $self->{_id};
> }
>
> # Accessor method for Reports _queue
> sub queue {
>         my ($self, $queue) = @_;
>         $self->{_queue} = $queue if defined($queue);
>         return $self->{_queue};
> }
>
> # Accessor method for Reports _owner
> sub owner {
>         my ($self, $owner) = @_;
>         $self->{_owner} = $owner if defined($owner);
>         return $self->{_owner};
> }
>
> # Accessor method for Reports _owner
> sub priority {
>         my ($self, $priority) = @_;
>         $self->{_priority} = $priority if defined($priority);
>         return $self->{_priority};
> }
>
> # Accessor method for Reports _owner
> sub worked {
>         my ($self, $worked) = @_;
>         $self->{_worked} = $worked if defined($worked);
>         return $self->{_worked};
> }
>
> # Accessor method for Reports _owner
> sub timeLeft {
>         my ($self, $timeLeft) = @_;
>         $self->{_timeLeft} = $timeLeft if defined($timeLeft);
>         return $self->{_timeLeft};
> }
>
> # Accessor method for Reports _owner
> sub due {
>         my ($self, $due) = @_;
>         $self->{_due} = $due if defined($due);
>         return $self->{_due};
> }
>
> # Accessor method for Reports _owner
> sub created {
>         my ($self, $created) = @_;
>         $self->{_created} = $created if defined($created);
>         return $self->{_created};
> }
>
> # Accessor method for Reports _owner
> sub updated {
>         my ($self, $updated) = @_;
>         $self->{_updated} = $updated if defined($updated);
>         return $self->{_updated};
> }
>
> # Accessor method for Reports _owner
> sub severity {
>         my ($self, $severity) = @_;
>         $self->{_severity} = $severity if defined($severity);
>         return $self->{_severity};
> }
>
> # Accessor method for Reports _owner
> sub ccl {
>         my ($self, $ccl) = @_;
>         $self->{_ccl} = $ccl if defined($ccl);
>         return $self->{_ccl};
> }


I see a pattern there. :)
These 11 methods are all doing basically the same, right?
So consider a set/get approach.

The "if defined($value)" parts are not really necessary, but it's good
that they are there: your class could, for example, be used in an
environment where dirtying the cache can hurt performance.
There can be one problem though: what if you want to change a property's
value to undef? :)

-- 
Affijn, Ruud

"Gewoon is een tijger."


-- 
To unsubscribe, e-mail: beginners-unsubscribe@perl.org
For additional commands, e-mail: beginners-help@perl.org
http://learn.perl.org/


