[Bioperl-l] For CVS developers -potentialpitfallwith"returnundef"

Heikki Lehvaslaiho heikki at sanbi.ac.za
Wed Jun 7 05:57:47 EDT 2006


Committed.

Please report any surprising changes in functionality to the list.

	-Heikki

On Tuesday 06 June 2006 10:04, Heikki Lehvaslaiho wrote:
> OK. I've gone through all cases where return and undef are on the same
> lines. I've done changes in 185 files.
>
> My aims have ben the following:
>
> 1. Remove undef from return undef when not necessary.
> 	This will make it easier to spot cases where undef matters in the future
> 	Most of the changes fall into this category. The context is clearly
> scalar.
>
> 2. Returning undef when user expects en empty list is bad
>
> ./Bio/Tools/Est2Genome.pm fixed
> ./Bio/SearchIO/blast.pm:2330:  should return (undef, undef) for clarity?
>                                not fixed
> ./Bio/Matrix/PSM/SiteMatrix.pm  fixed
> ./Bio/Matrix/PSM/Psm  fixed
> ./Bio/DB/Taxonomy::entrez.pm fixed
>
> 3. If docs say method returns nothing, explicit undef is not the right
> thing to return
>
> 4. do not return an explicit undef if the method is supposed to return
> false on failure
>
>
> Before I do the commit, I'd like to see number people to do 'make test' on
> bioperl-live and report back after the commit they see changes. There are
> quite a few tests that fail currently.
>
> I'll do the commit tomorrow Wednesday at 9 o'cock GMT.
>
> 	-Heikki
>
> On Friday 02 June 2006 09:52, Heikki Lehvaslaiho wrote:
> > I've started going through the files that have 'return undef' lines.
> > I'll report back later.
> >
> > Initial impression is that there are a few cases where the context
> > indicates list to be returned but failure returns an explicit undef. I'll
> > fix those.
> >
> > Most of the cases are much more ambiguous. Even when documentation says
> > the failure returns undef, it is clearly meant to mean false. In most
> > cases documentation does not comment on return value at all. Luckily the
> > context is almost always scalar and therefore it does not matter too
> > much.
> >
> > I seem to be changing 'return undef' to plain 'return' a bit
> > overzealously, so do not take it personally.
> >
> > 	-Heikki
> >
> > On Thursday 01 June 2006 19:46, Chris Fields wrote:
> > > ....
> > >
> > > > > Again, didn't do that.
> > > >
> > > > I'm very sorry that I allowed the ambiguity, but my comments were
> > > > certainly not directed at your recent changes to
> > > > Bio::Restriction::IO. In fact, I put in the above * comment to
> > > > exclude your changes from my discussion; you changed the docs because
> > > > the code never did what they said they did (the docs were bad).
> > > > That's fine (good!). My comments were a general point, slightly
> > > > directed at the idea of changing all the return undef;s - changing
> > > > the code so that it no longer matches the docs of a previously
> > > > working method. That's what I think is bad. Though in this particular
> > > > case it shouldn't make any difference at all.
> > >
> > > Agreed.  In any case, if tests have been properly set up then they
> > > should catch problems.  This is, of course, if they are properly set
> > > up.
> > >
> > > Chris
> > >
> > > > _______________________________________________
> > > > Bioperl-l mailing list
> > > > Bioperl-l at lists.open-bio.org
> > > > http://lists.open-bio.org/mailman/listinfo/bioperl-l
> > >
> > > _______________________________________________
> > > Bioperl-l mailing list
> > > Bioperl-l at lists.open-bio.org
> > > http://lists.open-bio.org/mailman/listinfo/bioperl-l

-- 
______ _/      _/_____________________________________________________
      _/      _/
     _/  _/  _/  Heikki Lehvaslaiho    heikki at_sanbi _ac _za
    _/_/_/_/_/  Associate Professor    skype: heikki_lehvaslaiho
   _/  _/  _/  SANBI, South African National Bioinformatics Institute
  _/  _/  _/  University of Western Cape, South Africa
     _/      Phone: +27 21 959 2096   FAX: +27 21 959 2512
___ _/_/_/_/_/________________________________________________________


More information about the Bioperl-l mailing list