From: Christophe de V. <cde...@al...> - 2003-02-07 15:07:51
|
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > ok. What are the remaining issues with the iterator proposal ? Could > you please sum up so we can get ahead with this ? > Personnaly I think that implement strictly one of the STL container idiom would be nice. Jonathan proposed sequence. Since it is quite simple and probably complete enough I think it is a reasonnable choice. On the other hand, the fact that the element type is itself a container may be problematic. So far I'm sure what would be the right thing. Christophe -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+Q8ujB+sU3TyOQjARAreHAJ0cJW2gjs4DX6H+aRTZSjsrfTFrEACgllvt +t1isLsWUCdwfExpkj3Dng8= =X5fb -----END PGP SIGNATURE----- |
From: Christophe de V. <cde...@al...> - 2003-05-22 14:13:47
|
=2D----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, I'd like to discuss the subject of iterators inside libxml++. We did already several month ago but this did not went to any=20 conclusion/decision. To illustrate how I would see things, I made a small patch which implements= a=20 class NodeIterator, which permit to iterate through the children of a node. I think it implements correctly the following concept from STL: =2D - Bidirectionnal Iterator. =2D - Input Iterator. It's quite trivial to use. Exemple to iterate a complete tree: void do_something(xmlpp::Node & node) { for( xmlpp::NodeIterator i =3D node.children_node(), i_end =3D xmlpp::NodeIterator(); i !=3D i_end; ++i) { std::cout << i->get_name() << std::endl; do_something(*i); } }; One interesting thing is that is nodes are added/removed at the same tree=20 level, the iterator will stay valid (unless we suppress the pointed node of= =20 course). A const iterator is needed, but I did not do it yet. I'd like to have comments/suggestions at least on: - Does this iterator have the behavior any C++ developer would expect ? - The class name. Should it be Node::iterator ? I'm not sure about that si= nce=20 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 ? - Should we wait for the next major version to add this feature ? The patch (which concerns only the 'nodes' directory) also includes a littl= e=20 change : it uses pimpl idiom so that no libxml header is included by the=20 libxml++ user.=20 I think it would be a good thing to do this for the whole library, and it's= =20 not much work. What do you think about it ? Comments welcome Cheers Christophe =2D----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+zNsQB+sU3TyOQjARAmhAAJ0f0HMTVUiJv9SrCoikIeprPYaJ+QCgxVgI 1dUmvUzJqx3rW09uoFJYfgE=3D =3DGMZg =2D----END PGP SIGNATURE----- |
From: Stefan S. <se...@sy...> - 2003-02-07 15:31:42
|
Christophe de VIENNE wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > >>ok. What are the remaining issues with the iterator proposal ? Could >>you please sum up so we can get ahead with this ? >> > > > Personnaly I think that implement strictly one of the STL container idiom > would be nice. > Jonathan proposed sequence. Since it is quite simple and probably complete > enough I think it is a reasonnable choice. It seems there are two ways to do this without creating too much overhead: 1) let the 'Node' object itself act as the container (which it actually is in libxml2), using 'child_iterator' and 'attribute_iterator'. 2) provide a thin 'NodeList' interface that provides the STL API. 'Node' would then have two methods 'NodeList get_children()' and 'NodeList get_attributes()'. 1) is what I currently propose, while 2) is a slight modification of what a) exists now and b) Jonathan suggests. The advantages over the original suggestion (just using an STL container 'NodeList') is that it could avoid unnecessary copying. So, while 2) still uses one (IMO) unnecessary indirection (you have to access the 'children' container to iterate it, instead of directly accessing the 'child_iterators') It seems to be quite reasonable performance-wise. Comments ? Stefan |
From: Jonathan W. <co...@co...> - 2003-02-07 17:36:27
|
On Fri, Feb 07, 2003 at 10:34:22AM -0500, Stefan Seefeld wrote: > It seems there are two ways to do this without creating too much > overhead: > > 1) let the 'Node' object itself act as the container (which it actually > is in libxml2), using 'child_iterator' and 'attribute_iterator'. > > 2) provide a thin 'NodeList' interface that provides the STL API. 'Node' > would then have two methods 'NodeList get_children()' and > 'NodeList get_attributes()'. > > > 1) is what I currently propose, while 2) is a slight modification of > what a) exists now and b) Jonathan suggests. The advantages over the > original suggestion (just using an STL container 'NodeList') is that it > could avoid unnecessary copying. So, while 2) still uses one (IMO) > unnecessary indirection (you have to access the 'children' container > to iterate it, instead of directly accessing the 'child_iterators') > It seems to be quite reasonable performance-wise. I'm now thinking that ForwardContainer migh be a better choice of STL concept to model, as it has fewer requirements so might be simpler to implement 1) while still supporting an STL-style interface. This would mean you can get an iterator from any Node, not just from some container type. I'm going to think more about this over the weekend, look at xmlwrapp again, and maybe look at how some examples of use would differ with the two proposals. jon -- "Fanaticism consists in redoubling your efforts when you have forgotten your aims." - George Santayana |
From: Stefan S. <se...@sy...> - 2003-02-07 17:58:34
|
Jonathan Wakely wrote: > I'm now thinking that ForwardContainer migh be a better choice of STL > concept to model, as it has fewer requirements so might be simpler to > implement 1) while still supporting an STL-style interface. This would > mean you can get an iterator from any Node, not just from some container > type. agreed. In fact, the more I think about suggestion 2) the more I like it: the 'Node' class has a 'get_children()' and a 'get_attribute()' method returning both a NodeList. So far it's almost as it was. However, instead of typedefing NodeList to be 'std::list<Node *>', I'd suggest we define it such that creation of a NodeList only implies a single copy: that of the first child node / attribute of the given node. The iterator is then implemented to traverse the libxml2 linked list, not the STL container as it did before. Does this sound satisfactory to everybody ? Stefan |
From: Christophe de V. <cde...@al...> - 2003-02-07 18:09:16
|
=2D----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Le Vendredi 7 F=E9vrier 2003 19:01, Stefan Seefeld a =E9crit : > Jonathan Wakely wrote: > > I'm now thinking that ForwardContainer migh be a better choice of STL > > concept to model, as it has fewer requirements so might be simpler to > > implement 1) while still supporting an STL-style interface. This would > > mean you can get an iterator from any Node, not just from some container > > type. > > agreed. In fact, the more I think about suggestion 2) the more I like > it: the 'Node' class has a 'get_children()' and a 'get_attribute()' > method returning both a NodeList. So far it's almost as it was. > However, instead of typedefing NodeList to be 'std::list<Node *>', > I'd suggest we define it such that creation of a NodeList only implies > a single copy: that of the first child node / attribute of the given > node. The iterator is then implemented to traverse the libxml2 linked > list, not the STL container as it did before. > > Does this sound satisfactory to everybody ? > Yes. We can already revert current iterators implementation on main branch. Then I'd like to see this as a patch before commiting something. For now I= =20 consider this more like a proposition (very interesting one though). Regards, Christophe =2D----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQE+Q/YsB+sU3TyOQjARArcYAJ9uGL9K5SHYQUsVlfm5kWQoTeVa1QCePo6V taCm/GXsYut9cPJfjphGdcw=3D =3Dbe21 =2D----END PGP SIGNATURE----- |