Project

Profile

Help

Forums » Help »

Code Review for TinyNodeImpl - setSystemId

Added by Anonymous over 16 years ago

Legacy ID: #4752516 Legacy Poster: hemartin (hemartin)

I do not suspect a bug in this, but maybe it is useful to look over it. The class TinyNodeImpl implements the method setSystemId(String) as: public void setSystemId(String uri) { short type = tree.nodeKind[nodeNr]; if (type == Type.ATTRIBUTE || type == Type.NAMESPACE) { getParent().setSystemId(uri); } else { tree.setSystemId(nodeNr, uri); } } As I figure it: the variable type will never be ATTRIBUTE or NAMESPACE. For these two types other arrays are defined in TinyTree. So I propose to write the method as: public void setSystemId(String uri) { tree.setSystemId(nodeNr, uri); } And override this method in TinyAttributeImpl: public void setSystemId(String uri) { getParent().setSystemId(nodeNr, uri); } The same applies for getSystemId(), which should also be overriden in TinyAttributeImpl. I do not know what to do with namespace nodes though. Hope this could help.


Replies (1)

RE: Code Review for TinyNodeImpl - setSystemI - Added by Anonymous over 16 years ago

Legacy ID: #4752763 Legacy Poster: Michael Kay (mhkay)

Thanks, yes, the code is indeed incorrect. If someone calls setSystemId() on an attribute node and the tree contains more attributes than element/text nodes, it could result in an array-index-out-of-bounds exception. In fact I think the correct action for an attribute node is probably to treat the call as a no-op. I think an attribute node should derive its base URI from its parent, not the other way around. There's no impact on namespace nodes because they are never implemented by TinyNodeImpl. Michael Kay http://www.saxonica.com/

    (1-1/1)

    Please register to reply