Forums

[As a note, I only just signed up for this list, and while I've received
one message that was a reply to Sara's original, it's not listed at

http://sbml.org/Forums/index.php?t=threadt&frm_id=3&rid=0

Perhaps the fact that it started off as a reply to an off-list message
messed up the forum software? At any rate, I'm replying blind a bit
here, since I can't see what you've been talking about in the last week.]

* Sarah Keating <skeating@caltech.edu> [2009-09-05 20:45] writes:
>
> I'm not sure why Neil thinks the functions are deprecated !

Presumably because:

> LibSBML for SBML L3 core and earlier levels will have this
> functionality. However, once libSBML becomes extendible we anticipate
> that package developers will provide code for their own package which
> will communicate with the core libSBML but not impact the code directly.
> Thus as packages introduce new elements derived from SBase the facility
> to setId/Name from the SBase class may be lost.

I had an idea about how one might satisfy all parties here, but instead of
trying to argue about it on this list, I instead just implemented it and
am attaching it as an svn diff to this email (hence the late-ish reply).

The basic idea is that, in an effort to try to satisfy both the 'new user'
issue:

> In libSBML-3 and earlier the id and name attributes were members of the
> base class SBase. This was a MISMATCH with the SBML specification. This
> inconsistency caused problems for new users. It also provided the
> opportunity to set an attribute on an element that did not in fact have
> that attribute - creating invalid SBML.

and the 'casting things is a pain in the butt' issue, the patch does the
following:

-'mId' and 'mName' are string members of SBase, and are "" by default.
-SBase::getId and SBase::getName returns those members (mod level 1
issues).
-SBase::isSetId / SBase::isSetName return false if mId / mName are "".
-SBase::setId and SBase::setName are virtual functions that return
LIBSBML_INVALID_ATTRIBUTE_VALUE and set nothing by default.

SBase subclasses now only need override 'setId' and 'setName' if id and
name are valid attributes of those elements.

Now, ideally, I'd have introduced 'SBaseId' and 'SBaseNoId' classes, but
this change seemed to be a decent compromise between matching the spec,
limiting re-used code (I deleted a *lot* of now-unnecessary 'getId' and
'getName' functions), and providing the very helpful 'getId' and 'getName'
functions on the SBase class.

(Another option is actually to simply eliminate the 'setId' and 'setName'
functions on SBase entirely; you have to balance providing documentation
vs. providing misleading code completion options.)

As new code is written for packages, nobody's old code need change at all
(you don't need to add new elements to your casting switch statement),
and there's a clear (and simple) method to follow for package code-writers
("implement 'setId' and 'setName'" if your element has those attributes").

At any rate, I understand if the team doesn't want to implement this
patch, but it was easy enough to code up and I was procrastinating from
doing something else anyway. I haven't exhaustively tested this, but it
compiles and links properly, and my sbml translator still produces the
same output when you throw the biomodels at it.

[Also, I left in the oddness with 'getId' and 'getName' with event
assignments, initial assignments, assignment rules, and rate rules. And I
couldn't find the Layout objects, so I didn't muck with them, either.]

-Lucian

Attachment: diff.txt (Size: 59.23KB, Downloaded 201 time(s))