
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,