[libxml++] node iterators

[Christophe de VIENNE <cdevienne alphacent com>]

-----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 
conclusion/decision.

To illustrate how I would see things, I made a small patch which implements a 
class NodeIterator, which permit to iterate through the children of a node.
I think it implements correctly the following concept from STL:
- - Bidirectionnal Iterator.
- - Input Iterator.

It's quite trivial to use. Exemple to iterate a complete tree:

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);
	}

};

One interesting thing is that is nodes are added/removed at the same tree 
level, the iterator will stay valid (unless we suppress the pointed node of 
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 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 ?
 - Should we wait for the next major version to add this feature ?



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 think it would be a good thing to do this for the whole library, and it's 
not much work. What do you think about it ?

Comments welcome


Cheers

Christophe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE+zNsQB+sU3TyOQjARAmhAAJ0f0HMTVUiJv9SrCoikIeprPYaJ+QCgxVgI
1dUmvUzJqx3rW09uoFJYfgE=
=GMZg
-----END PGP SIGNATURE-----

/* node.h
 * libxml++ and this file are copyright (C) 2000 by Ari Johnson, and
 * are covered by the GNU Lesser General Public License, which should be
 * included with libxml++ as the file COPYING.
 */

#include "nodeiterator.h"

#include "node.h"

#include "libxml++/exceptions/internal_error.h"

#include <libxml/tree.h>

namespace xmlpp
{
  NodeIterator::NodeIterator()
    : _impl(NULL)
    {
    }

  NodeIterator::NodeIterator(
      struct _xmlNode * p)
    : _impl(p)
  {
  }

  NodeIterator NodeIterator::operator++(int)
  {
    NodeIterator previous(*this);
    next();
    return previous;
  }

  NodeIterator & NodeIterator::operator++()
  {
    next();
    return *this;
  }

  NodeIterator NodeIterator::operator--(int)
  {
    NodeIterator previous(*this);
    prev();
    return previous;
  }

  NodeIterator & NodeIterator::operator--()
  {
    prev();
    return *this;
  }

  Node & NodeIterator::operator*() const
  {
    return *get();
  }

  Node * NodeIterator::operator->() const
  {
    return get();
  }
  
  bool NodeIterator::operator==(const NodeIterator & it) const
  {
    return _impl == it._impl;
  }
  
  bool NodeIterator::operator!=(const NodeIterator & it) const
  {
    return _impl != it._impl;
  }

  NodeIterator::operator bool () const
  {
    return _impl != NULL;
  }

  void NodeIterator::next()
  {
    if(_impl != NULL)
    {
      _impl = _impl->next;
    }
  }

  void NodeIterator::prev()
  {
    if(_impl->prev != NULL)
    {
      _impl = _impl->prev;
    }
  }

  Node * NodeIterator::get() const
  {
    if(_impl == NULL)
    {
      throw xmlpp::internal_error("");
    }
    return static_cast<Node*>( _impl->_private );
  }
};

/* nodeiterator.h
 * libxml++ and this file are copyright (C) 2000 by Ari Johnson, and
 * are covered by the GNU Lesser General Public License, which should be
 * included with libxml++ as the file COPYING.
 */

#ifndef __LIBXMLPP_NODES_NODEITERATOR_H
#define __LIBXMLPP_NODES_NODEITERATOR_H

extern "C" {
  struct _xmlNode;
};

namespace xmlpp
{
  class Node;

  class NodeIterator
  {
    private:
      struct _xmlNode * _impl;
    
    public:
      // compiler generate copy-constructor and destructor

      NodeIterator();
      NodeIterator(struct _xmlNode * p);


      NodeIterator operator++(int); // postincrement
      NodeIterator & operator++(); // preincrement

      NodeIterator operator--(int);
      NodeIterator & operator--();


      Node & operator*() const;
      Node * operator->() const;

      bool operator==(const NodeIterator & it) const;
      bool operator!=(const NodeIterator & it) const;

      operator bool () const;

    private:
      void next();
      void prev();
      Node * get() const;
  };
};


#endif // __LIBXMLPP_NODES_NODEITERATOR_H

? .deps
? .libs
? Makefile
? Makefile.in
? commentnode.lo
? element.lo
? libnodes.la
? node.lo
? nodeiterator.cc
? nodeiterator.h
? nodeiterator.lo
? nodeiterator.patch
? textnode.lo
Index: Makefile.am
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/Makefile.am,v
retrieving revision 1.1
diff -u -u -r1.1 Makefile.am
--- Makefile.am	18 Dec 2002 16:01:26 -0000	1.1
+++ Makefile.am	22 May 2003 14:00:46 -0000
@@ -1,7 +1,7 @@
 INCLUDES = -I$(top_srcdir) @LIBXML_CFLAGS@
 
-h_sources_public = node.h element.h commentnode.h textnode.h
-cc_sources = node.cc element.cc commentnode.cc textnode.cc
+h_sources_public = node.h element.h commentnode.h textnode.h nodeiterator.h
+cc_sources = node.cc element.cc commentnode.cc textnode.cc nodeiterator.cc
 
 
 noinst_LTLIBRARIES = libnodes.la
Index: commentnode.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/commentnode.cc,v
retrieving revision 1.4
diff -u -u -r1.4 commentnode.cc
--- commentnode.cc	7 Feb 2003 10:41:25 -0000	1.4
+++ commentnode.cc	22 May 2003 14:00:46 -0000
@@ -7,6 +7,8 @@
 #include <libxml++/nodes/commentnode.h>
 #include <libxml++/exceptions/internal_error.h>
 
+#include <libxml/tree.h>
+
 namespace xmlpp
 {
 
Index: commentnode.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/commentnode.h,v
retrieving revision 1.4
diff -u -u -r1.4 commentnode.h
--- commentnode.h	7 Feb 2003 10:41:25 -0000	1.4
+++ commentnode.h	22 May 2003 14:00:46 -0000
@@ -17,7 +17,7 @@
 class CommentNode : public Node
 {
 public:
-  explicit CommentNode(xmlNode* node);
+  explicit CommentNode(struct _xmlNode * node);
   virtual ~CommentNode();
 
   std::string get_content() const;
Index: node.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/node.cc,v
retrieving revision 1.21
diff -u -u -r1.21 node.cc
--- node.cc	22 May 2003 09:57:41 -0000	1.21
+++ node.cc	22 May 2003 14:00:46 -0000
@@ -5,6 +5,7 @@
  */
 
 #include <libxml/xpath.h>
+#include <libxml/tree.h>
 
 #include <libxml++/nodes/node.h>
 #include <libxml++/nodes/element.h>
@@ -25,6 +26,11 @@
 
 Node::~Node()
 {}
+
+NodeIterator Node::children_begin()
+{
+  return NodeIterator(_impl->children);
+}
 
 Node::NodeList Node::get_children(const std::string& name)
 {
Index: node.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/node.h,v
retrieving revision 1.18
diff -u -u -r1.18 node.h
--- node.h	22 May 2003 09:57:41 -0000	1.18
+++ node.h	22 May 2003 14:00:46 -0000
@@ -7,13 +7,18 @@
 #ifndef __LIBXMLPP_NODES_NODE_H
 #define __LIBXMLPP_NODES_NODE_H
 
+#include <libxml++/nodes/nodeiterator.h>
 #include <libxml++/noncopyable.h>
+
 #include <string>
 #include <list>
 #include <map>
 #include <vector>
 
-#include <libxml/tree.h>
+
+extern "C" {
+  struct _xmlNode;
+};
 
 namespace xmlpp {
 
@@ -33,7 +38,7 @@
   typedef std::list<Node*> NodeList;
   typedef std::list<Attribute*> AttributeList;
 
-  explicit Node(xmlNode* node);
+  explicit Node(struct _xmlNode * node);
   virtual ~Node();
 
   std::string get_name() const;
@@ -41,6 +46,7 @@
 
   int get_line() const;;
   
+  NodeIterator children_begin();
   NodeList get_children(const std::string& name = std::string());
   const NodeList get_children(const std::string& name = std::string()) const;
 
@@ -59,13 +65,13 @@
   NodeSet find(const std::string& xpath) const;
 
   ///Access the underlying libxml implementation.
-  xmlNode* cobj();
+  struct _xmlNode* cobj();
 
   ///Access the underlying libxml implementation.
-  const xmlNode* cobj() const;
+  struct _xmlNode const * cobj() const;
 
 private:
-   xmlNode* _impl;
+  struct _xmlNode* _impl;
 };
 
 } // namespace xmlpp
Index: textnode.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/textnode.cc,v
retrieving revision 1.5
diff -u -u -r1.5 textnode.cc
--- textnode.cc	7 Feb 2003 10:41:25 -0000	1.5
+++ textnode.cc	22 May 2003 14:00:46 -0000
@@ -6,7 +6,8 @@
 
 #include <libxml++/nodes/textnode.h>
 #include <libxml++/exceptions/internal_error.h>
-#include <iostream>
+
+#include <libxml/tree.h>
 
 namespace xmlpp
 {
Index: textnode.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/textnode.h,v
retrieving revision 1.5
diff -u -u -r1.5 textnode.h
--- textnode.h	7 Feb 2003 10:41:25 -0000	1.5
+++ textnode.h	22 May 2003 14:00:46 -0000
@@ -15,7 +15,7 @@
 class TextNode : public Node
 {
 public:
-  explicit TextNode(xmlNode* node);
+  explicit TextNode(struct _xmlNode* node);
   virtual ~TextNode();
 
   std::string get_content() const;