RE: [libxml++] node iterators



> From: Christophe de VIENNE [mailto:cdevienne alphacent com] 
> void do_something(xmlpp::Node & node) {
> 
> 	for( xmlpp::NodeIterator i = node.children_node(),
> 		i_end = xmlpp::NodeIterator();
> 		i != i_end;
> 		++i)
> 	{
> 		std::cout << i->get_name() << std::endl;
> 		do_something(*i);
> 	}
> 
> };

- I don't see begin() or end().
- Maybe xmlpp::NodeIterator should be xmlpp::Node::iterator or
xmlpp::Node::Children::iterator.

>  - The class name. Should it be Node::iterator ? I'm not sure 
> about that since 
> it would suggest Node is a container, which is not right I think.
>  - Does Node::children_begin() sounds right ? It seems to me that yes.
> 
>  - Should we drop Node::get_children ? simply keep it ?

I'm fairly convinced that the only way to do this is via another class, like
we do in some gtkmm widgets, such as Gtk::Model, Gtk::Container,
Gtk::Toolbar, and Gtk::ComboBox:

xmlpp::Node::ChildrenList& list = node.children();
for( xmlpp::Node::ChildrenList::iterator iter = list.begin(),
     iter != list.end();
     ++iter)
{
  std::cout << i->get_name() << std::endl;
  do_something(*i);
}

It's not just the implementation, but the interface itself. I don't think we
should mix an STL-style container API in with an unrelated API in one class.
The need for a children_ prefix seems to suggest that it should be a
separate class.

>  - Should we wait for the next major version to add this feature ?

Yes. We can freeze and branch soon if you like, though there are maybe these
open issues:
http://sourceforge.net/tracker/index.php?func=detail&aid=724953&group_id=129
99&atid=312999
http://sourceforge.net/tracker/index.php?func=detail&aid=708826&group_id=129
99&atid=312999

Also, we should wait a little to let the recent changes settle in. I haven't
had a chance to test the new release yet.

> The patch (which concerns only the 'nodes' directory) also 
> includes a little 
> change : it uses pimpl idiom so that no libxml header is 
> included by the 
> libxml++ user. 

I like not including the libxml headers. That's an aim of gtkmm too, re.
GTK+ headers.

I haven't actually read the patch yet.

Murray Cumming
murrayc usa net
www.murrayc.com 




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]