[Bioperl-l] For CVS developers - potential pitfallwith "returnundef"

[Lincoln Stein]

I'm afraid that everything depends on the context. If the subroutine is 
documented to return a single scalar, then returning undef is appropriate. If 
the subroutine is documented to return "false" on failure, then one must call 
return (or "return ()" ).

Changing all the return undefs to return is going to expose hidden bugs in the 
code written by people who are using BioPerl. While I agree wholeheartedly 
with the proposed audit, I think we need to expect that people are going to 
complain.

Lincoln


On Wednesday 31 May 2006 06:55, Heikki Lehvaslaiho wrote:
> In my opinion the sooner the bugs get exposed the better. It is much more
> likely that there is a well hidden bug caused by assigning accidentally
> undef into an one element array that someone intentionally writing code
> that expects that behaviour!
>
> I removed (but did not commit yet) all undefs from my old Bio::Variation
> code and could not see any differences in the test output.
>
> Let's remove them!
>
> 	-Heikki
>
> On Tuesday 30 May 2006 23:40, Chris Fields wrote:
> > Agreed, though I think these changes should be implemented at some point
> > (Conway's argument here makes sense and it is nice for Torsten to check
> > this out).  If proper tests are written then any changes resulting in
> > errors should be picked up by checking the appropriate test suite, though
> > I know it doesn't absolutely guarantee it.  ; P
> >
> > Chris
> >
> > > -----Original Message-----
> > > From: bioperl-l-bounces at lists.open-bio.org [mailto:bioperl-l-
> > > bounces at lists.open-bio.org] On Behalf Of Rutger Vos
> > > Sent: Tuesday, May 30, 2006 1:53 PM
> > > To: bioperl-l at lists.open-bio.org
> > > Subject: Re: [Bioperl-l] For CVS developers - potential pitfallwith
> > > "returnundef"
> > >
> > > Although I agree with the sentiment of following PBP, I'm not so sure
> > > changing 'return undef' to 'return' *now* will fix any bugs without
> > > introducing new, subtle ones.
> > >
> > > Chris Fields wrote:
> > > > Torsten,
> > > >
> > > > Any way you can post a list of some/all of the offending lines or
> > >
> > > modules?
> > >
> > > > Sounds like something to consider, but if the list is as large as you
> > >
> > > say we
> > >
> > > > made need something (bugzilla? wiki?) to track the changes and make
> > > > sure they pass tests; I'm sure a large majority will.
> > > >
> > > > I'm guessing Jason would want this somewhere on the project priority
> > >
> > > list or
> > >
> > > > bugzilla, with a link to the actual list, but I'm not sure.  Maybe
> > > > start
> > >
> > > a
> > >
> > > > page on the wiki for proposed code changes?
> > > >
> > > > Chris
> > > >
> > > >> -----Original Message-----
> > > >> From: bioperl-l-bounces at lists.open-bio.org [mailto:bioperl-l-
> > > >> bounces at lists.open-bio.org] On Behalf Of Torsten Seemann
> > > >> Sent: Tuesday, May 30, 2006 3:19 AM
> > > >> To: bioperl-l at lists.open-bio.org
> > > >> Subject: [Bioperl-l] For CVS developers - potential pitfall with
> > > >> "returnundef"
> > > >>
> > > >> FYI Bioperl developers:
> > > >>
> > > >> I just audited the bioperl-live CVS and found about 450 occurrences
> > > >> of "return undef".
> > > >>
> > > >> Page 199 of "Perl Best Practices" by Damian Conway, and this URL
> > > >> http://www.perl.com/lpt/a/2006/02/23/advanced_subroutines.html
> > > >> suggest:
> > > >>
> > > >> "Use return; instead of return undef; if you want to return nothing.
> > > >> If someone assigns the return value to an array, the latter creates
> > > >> an array of one value (undef), which evaluates to true. The former
> > > >> will correctly handle all contexts."
> > > >>
> > > >> So I'm guessing at least some of these 450 occurrences *could*
> > > >> result
> > >
> > > in
> > >
> > > >> bugs and should probably be changed.
> > > >>
> > > >> Your opinion may differ :-)
> > > >>
> > > >> --
> > > >> Dr Torsten Seemann               http://www.vicbioinformatics.com
> > > >> Victorian Bioinformatics Consortium, Monash University, Australia
> > > >>
> > > >> _______________________________________________
> > > >> 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
> > >
> > > --
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > Rutger Vos, PhD. candidate
> > > Department of Biological Sciences
> > > Simon Fraser University
> > > 8888 University Drive
> > > Burnaby, BC, V5A1S6
> > > Phone: 604-291-5625
> > > Fax: 604-291-3496
> > > Personal site: http://www.sfu.ca/~rvosa
> > > FAB* lab: http://www.sfu.ca/~fabstar
> > > Bio::Phylo: http://search.cpan.org/~rvosa/Bio-Phylo/
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >
> > >
> > > _______________________________________________
> > > 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

-- 
Lincoln D. Stein
Cold Spring Harbor Laboratory
1 Bungtown Road
Cold Spring Harbor, NY 11724
(516) 367-8380 (voice)
(516) 367-8389 (fax)
FOR URGENT MESSAGES & SCHEDULING, 
PLEASE CONTACT MY ASSISTANT, 
SANDRA MICHELSEN, AT michelse at cshl.edu