Quoting Robert Crosbie <swingincelt@...400...>:
There are some wrappers that I'm not sure what to do with:
[sp_repr_add_child] returns a value, but it is never checked. I assume a straight replacement is fine in this case and ignore the return value.
Yes.
There are others like [sp_repr_remove_child], where the value is actually checked somewhere. Should the ckeck be removed since it will always be true anyway?
Yes.
The return values were orginally to indicate success/failure, but the various methods can no longer fail (if they can fail again in the future, then they will do so by throwing an exception).
Should all instances of append_child be replaced with addChild? There are many, but it could be a Janitor item.
No, use appendChild.
repr->appendChild(child) is guaranteed to have performance equal to or better than repr->addChild(child, repr->lastChild()).
void addChild(Inkscape::XML::Node *child, Inkscape::XML::Node *ref); void appendChild(Inkscape::XML::Node *child) { addChild(child, _last_child); }
Please note that Inkscape::XML::SimpleNode is only one possible implementation of the SPRepr (now Inkscape::XML::Node) interface. Just because it implements appendChild in terms of addChild doesn't mean others will (or that it will continue to do so in the future).
(I realize I do need to document Inkscape::XML::Node itself...)
By the way -- I want to keep sp_repr_parent(), sp_repr_next() (and sp_repr_prev()), though I will be renaming them into the Inkscape::XML namespace relatively soon. There are a few places where using those instead of direct method calls simplifies logic, as e.g. sp_repr_parent(NULL) is well-defined.
-mental