[Bioperl-l] best coding practices

[Hilmar Lapp]

Historically, we used to write getter/setters in the following style:

sub some_property{
	my ($self,$val) = @_;
	if(defined($val)) {
	   $self->{someproperty} = $val;
	}
	return $self->{someproperty};
}

You could safely delegate to methods implemented in this style using 
the same style without any further precaution:

sub some_derived_property{
	my ($self,$val) = @_;
	# this is for the example only. DO NOT DO THIS, SEE BELOW WHY
	return $self->some_property($val);
}

Some people wrote this in a slightly different way, but the (bad) 
result is essentially the same:

sub some_derived_property{
	my $self = shift;
	# this is for the example only. DO NOT DO THIS EITHER, SEE BELOW WHY
	return $self->some_property(shift);
}

Because our traditional implementation would not allow unsetting the 
value to undef, we changed the template to the following style:

sub some_property{
     my $self = shift;
     return $self->{'someproperty'} = shift if @_;
     return $self->{'someproperty'};
}

Many new and also quite a number of old getter/setters are now 
implemented using the new style especially if unsetting the value shall 
be permitted.

Delegating to a method implemented in the new style using the examples 
given above is *bad* because it results in accidental unsetting of the 
value!

If you delegate to a method, *always* use the following style or 
something similar. It is safe regardless in which style the delegation 
target is implemented. It is also forward compatible if the method to 
which you delegate will ever expect or deal with more than just one 
argument (or change to named parameters for that matter).

sub some_derived_property{
	my $self = shift;
	# this is the Right Way to delegate: @_ is an empty array if exhausted 
after
	# pulling off $self, not an undefined but present argument
	return $self->some_property(@_);
}

I had to clean up a couple of methods in Bio::Search::HSP::HSPI and it 
took me quite a while to track down the offender (BTW HSPI does a bit 
more than just decorating implementations ...).

I don't know whose coding style using blabla(shift) for delegation is, 
but Steve or Jason please when you get a chance it'd be great if you go 
through all of those Search modules where this applies. Every such call 
is a lurking bug waiting to be triggered by changing the underlying 
implementation of the delegation target.

	-hilmar

-- 
-------------------------------------------------------------
Hilmar Lapp                            email: lapp at gnf.org
GNF, San Diego, Ca. 92121              phone: +1-858-812-1757
-------------------------------------------------------------