[Bioperl-l] Bio::Ontology::Term (rollback question)

[Chris Fields]

On Aug 27, 2007, at 6:07 PM, Hilmar Lapp wrote:

> The B::O::TermI interface actually says that get_dblinks() would  
> return scalars. That's why the add_dblink methods accept strings. I  
> also agree that this is inconsistent with with the rest of BioPerl.
>
> Oddly enough, Term::add_dblink_context() does ask for DBLink  
> objects, though it doesn't seem to be enforced, even though  
> Term::get_dblink_context() is advertised as returning scalars.

This happened b/c of stringification and 'eq' overloading.  Just  
removing the overloads didn't reveal this problem; I had to add  
exceptions to them to fish this out.

> So it does seem this is messed up design-wise. It seems to me that  
> to really fix this would inevitably break the API, and I don't see  
> how you would make this backwards compatible w/o creating a lot of  
> messy code, the sole purpose of which would be backwards  
> compatibility.
>
> One could only fix Term::add_dblink_context() as it's not in the  
> interface but that wouldn't contribute anything to improving  
> consistency.

Agreed; in fact it may make it more confusing.

> So the alternative to breaking the API in a non-backwards  
> compatible fashion would be to add to it, map the existing dblink  
> methods onto the added ones, and start deprecating them. For  
> example, you could add methods get_dbxrefs() (also on the  
> interface), add_dbxref(), etc,   and build in a context argument so  
> we don't need another set of methods for that. They would accept  
> and return DBLink objects, and the get_dblink() methods could be  
> changed to map those to scalars while also getting slated for  
> deprecation.
>
> Does this make sense?
>
> 	-hilmar

I think so; I'll have to look over the code to see how we would  
implement this, though I'm guessing everything would be stored as  
DBLink objects by default.  Any changes will probably need to wait  
until after I fish out any remaining spots in the code where  
overloading is being used, but at least we have a direction on where  
to go.

chris