[Biojava-l] Nasty CompoundLocation `issue' -- incompatible change

[Thomas Down]

On Fri, Jun 01, 2001 at 07:36:04AM -0400, Michael Dickson wrote:
> I'd like to chime in and say that I'm not crazy about the approach 
> either.  Static methods on singletons that behave as factories is one 
> thing but in an implementation class I feel that the contstructor (or a 
> seperate initializer method in complex cases like this one) are a better 
> choice. 
> 
> We get alot of cool stuff from BioJava so its not a major issue but I 
> did want to weigh in with my opinion.

The issue is that, currently, the CompoundLocation constructor
requires that the following all be true:

  - The list of Locations is sorted into natural order.

  - They're all contiguous.
 
  - They're all non-overlapping.

If any of these aren't true then the constructed object will,
/silently/ fail to meet the full contract specified by the
Location interface.  Thus it's too dangerous to be a public
method.  The two options are:

  - The solution I adopted, of having an n-ary union operator 
    on LocationTools.

  - Make the constructor perform the sort and block merge
    itself.

The code is almost identical, either way.  My main concern with
putting it on the constructor is that, after sorting and mergin
a list of locations, you may end up with everything collapsing
down to a single, contiguous, block.  In this case you don't
really want a CompoundLocation at all, you want a RangeLocation.

There would also be a performance penalty added to the
(common) binary union operator.

My real feeling is that CompoundLocation shouldn't be a public
class at all.  If it disappeared, and you just created them via
the two union methods on LocationTools, would you feel happier.



All this said, this really seems like a minor issue -- I was
just fixing a (rather nasty) bug the way which seemed simplest
to me.  If there's really demand it's easy enough to change
it back.

    Thomas.