removing trivial wrappers
There are some wrappers that I'm not sure what to do with:
This one below returns a value, but it is never checked. I assume a straight replacement is fine in this case and ignore the return value. inline unsigned int sp_repr_add_child(Inkscape::XML::Node *repr, Inkscape::XML::Node *child, Inkscape::XML::Node *ref) { repr->addChild(child, ref); return true; }
There are others like this one below, where the value is actually checked somewhere. Should the ckeck be removed since it will always be true anyway? inline unsigned int sp_repr_remove_child(Inkscape::XML::Node *repr, Inkscape::XML::Node *child) { repr->removeChild(child); return true; }
... ok = sp_repr_remove_child (NODE_DATA (old_parent)->repr, NODE_DATA (node)->repr) && sp_repr_add_child (NODE_DATA (new_parent)->repr, NODE_DATA (node)->repr, ref); ...
sp_repr_set_attr is another one that always returns true, but is used as a return value and in conditionals in a number of places.
Should all instances of append_child be replaced with addChild? There are many, but it could be a Janitor item.
void addChild(Inkscape::XML::Node *child, Inkscape::XML::Node *ref); void appendChild(Inkscape::XML::Node *child) { addChild(child, _last_child); }
Cheers,
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
participants (2)
-
unknown＠example.com -
Robert Crosbie