Re: Proper class setup?

Dr.Ruud wrote:
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]

Duly noted :D


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.


I'm not sure what you mean by "Change your new() in a new() and init()".

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.


What do you mean by "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? :)

.

Relevant Pages