GNOME Bugzilla – Bug 338907
"const_NodeList" instead of "const NodeList"
Last modified: 2015-09-17 09:26:26 UTC
the public member: const NodeList Node::get_children (const Glib::ustring &name) const being NodeList typedef std::list< Node * > NodeList causes problems in C++ when using const qualifiers properly. the right way of having the API is: const_NodeList Node::get_children (const Glib::ustring &name) const being const_NodeList typedef std::list< const Node * > const_NodeList -------------------- example: class foo { void load(const xmlpp::Node* anode) { } void load_doc() { ... const xmlpp::Node::NodeList childs=root_element.get_children("child"); load(*childs.begin())); //will fail because is passing a Node* to load //instead a const Node* } }; proper way: class foo { void load(const xmlpp::Node* anode) { } void load_doc() { ... xmlpp::Node::const_NodeList childs=root_element.get_children("child"); load(*childs.begin())); //passing a const Node* to load, ok //but i'm longing for the const_NodeList type 8-(( } };
I'm aware that this is less than ideal. However, you could still add/remove nodes to a std::list< const Node * > and we would prefer to prevent that, to make it clearer that doing so would have no effect. I wonder, can we actually call non-const methods on the elements of the current const std::list< Node * > ?
as i see, the list actually returned (const list<Node*>) is a copy, there shouldn't exist a reason to prevent the caller of modifying it in any manner. Maybe she wants to use this nodelist for calling a third party function and she wants to add some extra nodes to the list, or remove some of them,..., whatever she did with the list is safe for libxml++, because is her own copy. But, what if she access an element of the list, which is a pointer to data stored in libxml++ internal structures, and change something?, it is a Node, not a const Node*, so she might be able to destroy data and damage the libxml++ operation. So, if there's something to prevent, they are the pointers to internal structures. I need a Node to be readed, so I want a const Node*, The API can't provide me with that, so I have to use const_cast stuff which leaves my code pffff. anyway, you could compare with stl... "iterator", "const_iterator"... (not "const iterator") regards
a forgot to answer your last question: yes, of course you can call non const methods to a Node* theres another solution which might satify to both: const std::list<const Node*> but I recomend std::list<const Node*> because of my last comment regards
Created attachment 296780 [details] [review] patch: Node::get_children(): Propagate const qualifier to children An API breaking patch for Node::get_children(). I agree with Marcos that the const version of Node::get_children() shall be typedef std::list<const Node*> const_NodeList; const_NodeList get_children(const Glib::ustring& name = Glib::ustring()) const; Compare std::vector<Gtk::Widget*> Gtk::Container::get_children(); std::vector<const Gtk::Widget*> Gtk::Container::get_children() const; I don't understand the example in comment 0. A Node* can be assigned to a const Node*, but a const Node* can't be assigned to a Node*. The problem with the present constness handling is not so much that it's impossible to do reasonable things, but rather that it's possible to do unreasonable things.
Created attachment 296781 [details] [review] patch: Node::find(): Propagate const qualifier to found nodes A similar API breaking patch for Node::find(). When it's time to break API, we should consider moving the find() and eval_*() methods from Node to Document. They search the whole document to which the node belongs.
Created attachment 296782 [details] [review] patch: Element::get_attributes(): Propagate const qualifier to attributes A similar API breaking patch for Element::get_attributes(). See also bug 632522 and bug 632524. They also contain API breaking patches that fix constness bugs.
(In reply to Kjell Ahlstedt from comment #5) > When it's time to break API, we should consider moving the find() and > eval_*() > methods from Node to Document. They search the whole document to which the > node > belongs. Sorry, this is not true. An XPath can be either relative to the current node, or absolute (= relative to the document's root node). Like file paths. find() and eval_*() shall remain members of Node.
I have pushed updated versions of the 3 patches + a patch that replaces xmlpp:NodeSet by xmlpp::Node::NodeSet.