[Bioperl-l] Inconsitency in return value ofBio::SimpleAlinment::remove_columns()

Seiji Kumagai skumagai at life.bio.sunysb.edu
Fri Sep 22 20:03:36 UTC 2006

On Fri, 22 Sep 2006, Chris Fields wrote:

>> I, at the same time, noticed another issue with the method. According to
>> the POD, the method returns a new alignment. However, iff argument is not
>> passed to it, the return value is $self. I see this may be a problem
>> because modifications on the returned value affect the original object
>> only if this method is called in the way I mentioned. Secondly, it is just
>> confusing to me that returned value is different but superficially
>> identical (both are Bio::SimpleAlignment object).
>> I want to make a modification on this issue by one of the following ways:
>> 1) Make an argument mandatory. If argument is missing, throw an
>> exception. It is user's responsibility to handle it properly.
>> 2) Always return new SimpleAlign object. If the argument is not passed,
>> return a clone of $self. This may make the scripts run slower and use more
>> memory when no argument is given, but modification on new object is
>> guaranteed not to affect original object.
>> 3) Do not modify current version.
>> 3.a) Do not modify current version, but state the difference in POD.
>> What do you think? I personally think the first is the best since calling
>> the method but doing nothing is not what users want to do in most of the
>> cases. Those rare cases are deserved to be handled by an exception.
>> Thanks,
> The first option sounds appropriate considering the use of the method in
> POD.  I also agree that returning $self isn't the best practice either,
> since it would not be a true clone of the object (unless there is some deep
> copy magic going on that I don't see).
> What would you suggest?

Unless someone says otherwise, I think the following is enough for the 
first option.

Index: SimpleAlign.pm
RCS file: /home/repository/bioperl/bioperl-live/Bio/SimpleAlign.pm,v
retrieving revision 1.110
diff -B -b -u -r1.110 SimpleAlign.pm
--- SimpleAlign.pm      21 Sep 2006 19:20:12 -0000      1.110
+++ SimpleAlign.pm      22 Sep 2006 23:17:23 -0000
@@ -987,7 +987,10 @@

  sub remove_columns {
         my ($self, at args) = @_;
-       @args || return $self;
+       @args || $self->throw
+            (-class => 'Bio::Root::BadParameter',
+             -text => 'Mandatory parameter is missing. see 
+             -value 'array reference');
     my $aln;

         if ($args[0][0] =~ /^[a-z_]+$/i) {

Also, I noticed remove_columns() uses or claims to use a coordinate system 
starting from 0, whereas slice() uses a system from 1. Isn't it better to 
have the same scheme?


More information about the Bioperl-l mailing list