Re: [Libxmlplusplus-general] parametrizing libxml++ for the character /string type

[Stefan Seefeld <seefeld sympatico ca>]

Christophe de VIENNE wrote:

> I'm currently not having the time to do this, but your patches are very 
> welcome. However I won't put them in the current branch but in the unstable 
> one (which does not exists yet but as soon as it is needed, it will) as 
> Murray said.

ok, here is the first patch. The goal was to change the implementation to
delegate whatever we can down to libxml2, while respecting the existing API.

So, everything compiles, and the examples run unchanged.

There are, however, a couple of issues which need to be addressed. I hope we can
sort them out together. First a little account on what this change does:

All libxml2 structures use '_private' members for application data. I use that
to point to the corresponding libxml++ wrapper class, so we can do a reverse
lookup. For example, to access the first child node of a xmlpp::Node object,
you'd do something like:

reinterpret_cast<Node *>(this->_impl->children->_private)

Easy enough, isn't it ?

The tricky point is, as said earlier, ownership management. libxml2's nodes
are owned by their parent nodes (and ultimately by the enclosing document),
not by the libxml++ wrapper object. We need to work out how transfer of ownership
should happen when a node is unlinked from its document / parent node.

Another tricky point is that libxml2 will automagically merge nodes occasionally,
for example if you insert a new text node right after an existing text node.
Thus,

Node *Node::add_child(const std::string &)

may or may not return a new object. It is, however, (and luckily,) owned by the
parent node, so the caller doesn't have to care. A similar argument is to be made
for setting attributes.

All this said, I believe there are now a couple of ways to enhance the API itself:

* I'd like to add iterators to make child node and attribute traversal more efficient
  (right now they are copied into a temporary container that is returned)

* I'd like to suggest to add a 'Visitor' interface for simpler traversal of a document,
  notably to externalize it (the 'write' method would look a *lot* simpler)

* the domparser should be refactored into a 'Document' and possibly a single
  'Document *parse_document(std::istream &)' factory function.

* add new node types such as 'processing instruction', 'cdata section', etc.

* new functionality can be added (notably the xpath lookup stuff I have been suggesting)

* do the split into character type agnostic/specific parts, and hook up external
  unicode libraries

Anyways, I guess that's enough for tonight :-)
Let me know what you think of this plan...

Enjoy,
		Stefan

Index: libxml++/attribute.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/attribute.cc,v
retrieving revision 1.6
diff -u -r1.6 attribute.cc
--- libxml++/attribute.cc	26 Dec 2002 09:06:52 -0000	1.6
+++ libxml++/attribute.cc	29 Jan 2003 03:33:51 -0000
@@ -9,31 +9,31 @@
 namespace xmlpp {
 
 Attribute::Attribute(const std::string& name, const std::string& value)
-: _name(name), _value(value)
-{ }
+   : _impl(xmlNewProp(0, (xmlChar *)name.c_str(), (xmlChar *)value.c_str()))
+{
+   _impl->_private = this;
+}
 
 Attribute::Attribute(xmlAttrPtr attr)
+   : _impl(attr)
 {
-  if(attr->name)
-    _name = (const char*) attr->name;
-
-  if(attr->children)
-    _value = (const char*) attr->children->content;
+   _impl->_private = this;
 }
 
 std::string Attribute::get_name() const
 {
-  return _name;
+   return (char *)_impl->name;
 }
 
 std::string Attribute::get_value() const
 {
-  return _value;
+  return (char *)_impl->children;
 }
 
 void Attribute::set_value(const std::string& value)
 {
-  _value = value;
+  _impl = xmlSetProp(_impl->parent, _impl->name, (xmlChar *)value.c_str());
+  _impl->_private = this;
 };
 
 
Index: libxml++/attribute.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/attribute.h,v
retrieving revision 1.7
diff -u -r1.7 attribute.h
--- libxml++/attribute.h	19 Dec 2002 12:37:49 -0000	1.7
+++ libxml++/attribute.h	29 Jan 2003 03:33:51 -0000
@@ -28,8 +28,7 @@
   void set_value(const std::string& value);
 
 private:
-  std::string _name;
-  std::string _value;
+  xmlAttr *_impl;
 };
 
 } // namespace xmlpp
Index: libxml++/nodes/element.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/element.cc,v
retrieving revision 1.4
diff -u -r1.4 element.cc
--- libxml++/nodes/element.cc	26 Dec 2002 09:06:52 -0000	1.4
+++ libxml++/nodes/element.cc	29 Jan 2003 03:33:51 -0000
@@ -18,32 +18,15 @@
 Element::Element(const Element* from)
   : Node(from)
 {
-  const AttributeList& attributes = from->get_attributes();
-  for(AttributeList::const_iterator curattr = attributes.begin(); curattr != attributes.end(); ++curattr)
-  {
-    add_attribute((*curattr)->get_name(), (*curattr)->get_value());
-  }
 }
 
 Element::Element(xmlNodePtr node)
   : Node(node)
 {
-  for(xmlAttrPtr attr = node->properties;
-		  attr;
-		  attr = attr->next)
-  {
-	  add_attribute(attr);
-  }
 }
 
 Element::~Element()
 {
-  for(AttributeList::iterator i = _attributes_list.begin();
-		  i != _attributes_list.end();
-		  ++i)
-  {
-    delete *i;
-  }
 }
 
 void Element::write(xmlDocPtr doc, xmlNodePtr parent) const
@@ -63,99 +46,28 @@
 
   if(node)
   {
-    //Add the child nodes:
-    for(NodeList::const_iterator iter = _children.begin();
-       iter != _children.end();
-       ++iter)
-    {
-      //Add the nodes to this parent node:
-
-      //write() is a virtual function, implemented differently for each node type.
-      (*iter)->write(doc, node);
-    }
-
-    //Add the attributes:
-    for(AttributeList::const_iterator iter = _attributes_list.begin();
-      iter != _attributes_list.end();
-      ++iter)
-    {
-      const Attribute* attr = *iter;
-      xmlSetProp(node, (xmlChar*)attr->get_name().c_str(), (xmlChar*)attr->get_value().c_str());
-    }
+     NodeList children = get_children();
+     for(NodeList::const_iterator iter = children.begin();
+         iter != children.end();
+         ++iter)
+     {
+        //Add the nodes to this parent node:
+        
+        //write() is a virtual function, implemented differently for each node type.
+        (*iter)->write(doc, node);
+     }
+     
+     AttributeList attributes = get_attributes();
+     for(AttributeList::const_iterator iter = attributes.begin();
+         iter != attributes.end();
+         ++iter)
+     {
+        const Attribute* attr = *iter;
+        xmlSetProp(node, (xmlChar*)attr->get_name().c_str(), (xmlChar*)attr->get_value().c_str());
+     }
   }
 
 }
-
-Attribute* Element::get_attribute(const std::string& name) {
-  AttributeMap::iterator i = _attributes_map.find(name);
-  if(i == _attributes_map.end())
-  {
-    return NULL;
-  }
-  else
-  {
-    return i->second;
-  }
-}
-
-Attribute* Element::add_attribute(const std::string& name, const std::string& value) {
-  Attribute* tmp = 0;
-
-  if(_attributes_map.find(name) != _attributes_map.end())
-    return NULL;
-
-  tmp = new Attribute(name, value);
-
-  if(!tmp)
-    return NULL;
-
-  _attributes_map[tmp->get_name()] = tmp;
-  _attributes_list.insert(_attributes_list.end(), tmp);
-
-  return tmp;
-}
-
-Attribute* Element::add_attribute(xmlAttrPtr attr)
-{
-	if(_attributes_map.find( (const char*)attr->name ) != _attributes_map.end())
-	{
-		return 0;
-	}
-
-	Attribute* tmp = 0;
-
-	if(tmp = new Attribute(attr))
-	{
-		_attributes_map[tmp->get_name()] = tmp;
-		_attributes_list.insert(_attributes_list.end(), tmp);
-	}
-
-	return tmp;
-}
-
-void Element::remove_attribute(const std::string& name) {
-  AttributeMap::iterator i = _attributes_map.find(name);
-  if(i != _attributes_map.end())
-  {
-    Attribute* attribute = i->second;
-    _attributes_list.remove(attribute);
-    _attributes_map.erase(i);
-
-    delete attribute;
-  }
-}
-
-const Element::AttributeList& Element::get_attributes() const
-{
-  return _attributes_list;
-}
-
-const Attribute* Element::get_attribute(const std::string& name) const
-{
-  return const_cast<Element*>(this)->get_attribute(name);
-}
-
-
 
 } // namespace xmlpp
 
Index: libxml++/nodes/element.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/element.h,v
retrieving revision 1.3
diff -u -r1.3 element.h
--- libxml++/nodes/element.h	25 Dec 2002 01:25:07 -0000	1.3
+++ libxml++/nodes/element.h	29 Jan 2003 03:33:51 -0000
@@ -10,40 +10,24 @@
 #include <libxml++/nodes/node.h>
 #include <libxml++/attribute.h>
 
-
 namespace xmlpp {
 
-class TextNode;
-
 class Element : public Node
 {
 public:
-  typedef std::list<Attribute*> AttributeList;
-  typedef std::map<std::string, Attribute*> AttributeMap;
-
   explicit Element(const std::string& name);
   explicit Element(const Element* from);
   explicit Element(xmlNodePtr node);
   virtual ~Element();
 
-  const AttributeList& get_attributes() const;
-  Attribute* get_attribute(const std::string& name);
-  const Attribute* get_attribute(const std::string& name) const;
-  Attribute* add_attribute(const std::string& name, const std::string& value = std::string());
-
-  /** Remove the child node.
-   * @param node The child node to remove. The Attribute will be deleted by this method, so any references to it will be unusable.
-   */
-  void remove_attribute(const std::string& name);
+  using Node::get_attributes;
+  using Node::get_attribute;
+  Attribute *add_attribute(const std::string &name, const std::string &value)
+  { return set_attribute(name, value);}
+  using Node::set_attribute;
+  using Node::remove_attribute;
 
   virtual void write(xmlDocPtr doc, xmlNodePtr parent = 0) const;
-
-private:
-  Attribute* add_attribute(xmlAttrPtr attr);
-
-protected:
-  AttributeList _attributes_list;
-  AttributeMap _attributes_map;
 };
 
 } // namespace xmlpp
Index: libxml++/nodes/node.cc
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/node.cc,v
retrieving revision 1.6
diff -u -r1.6 node.cc
--- libxml++/nodes/node.cc	25 Dec 2002 01:25:07 -0000	1.6
+++ libxml++/nodes/node.cc	29 Jan 2003 03:33:52 -0000
@@ -14,73 +14,65 @@
 namespace xmlpp {
 
 Node::Node(const std::string& name)
- : _name(name), _line(0)
+   : _impl(xmlNewNode(0, reinterpret_cast<const xmlChar*>(name.c_str())))
 {
+   _impl->_private = this;
 }
 
 
 Node::Node(const Node* from)
-: _name(from->get_name()), _line(from->get_line())
+   : _impl(xmlCopyNode(from->_impl, 1))
 {
-  const NodeList& children = from->get_children();
-  for(NodeList::const_iterator curnode = children.begin(); curnode != children.end(); ++curnode)
-  {
-    //TODO: Create the appropriate type:
-    add_child(new Node(*curnode));
-  }
+   _impl->_private = this;
+    for (xmlNode *child = _impl->children; child; child = child->next)
+    {
+      switch (child->type)
+      {
+        case XML_ELEMENT_NODE: child->_private = new Element(child); break;
+        case XML_TEXT_NODE: child->_private = new TextNode(child); break;
+        case XML_COMMENT_NODE: child->_private = new CommentNode(child); break;
+        // more types to come...
+      }
+    }
+    for (xmlAttr *attr = _impl->properties; attr; attr = attr->next)
+      attr->_private = new Attribute(attr);
 }
 
 Node::Node(xmlNodePtr node)
+   : _impl(node)
 {
-  if(node->name)
-    _name = (char*) node->name;
-
-  _line = XML_GET_LINE(node);
-
-  //Add child nodes:
-  for(xmlNodePtr child = node->children;
-		  child;
-		  child = child->next)
-  {
-    Node* pChild = 0;
-
-    //TODO: Deal with more types:
-    if(child->type == XML_TEXT_NODE)
-      pChild = new TextNode(child);
-    else if(child->type == XML_COMMENT_NODE)
-      pChild = new CommentNode(child);
-    else
-      pChild = new Element(child);
-    
-	  add_child(pChild);
-  }
+   _impl->_private = this;
+    for (xmlNode *child = _impl->children; child; child = child->next)
+    {
+      switch (child->type)
+      {
+        case XML_ELEMENT_NODE: child->_private = new Element(child); break;
+        case XML_TEXT_NODE: child->_private = new TextNode(child); break;
+        case XML_COMMENT_NODE: child->_private = new CommentNode(child); break;
+        // more types to come...
+      }
+    }
+    for (xmlAttr *attr = _impl->properties; attr; attr = attr->next)
+      attr->_private = new Attribute(attr);
 }
 
-Node::~Node() {
-  NodeList::iterator curchild;
-
-  for(NodeList::iterator i = _children.begin();
-		  i != _children.end();
-		  ++i)
-  {
-    delete *i;
-  }
+Node::~Node()
+{
 }
 
 Node::NodeList Node::get_children(const std::string& name)
 {
-  if(name.empty())
-    return _children;
-  else
-  {
-    //Return only the nodes with the specified name:
-    NodeList retval;
-    for(NodeList::const_iterator cur = _children.begin(); cur != _children.end(); cur++)
-      if((*cur)->get_name() == name)
-        retval.insert(retval.end(), *cur);
+   xmlNode *child = _impl->children;
+   if (!child) return NodeList();
 
-     return retval;
-  }
+   NodeList children;
+   do
+   {
+      if (name.empty() || name == (char *)child->name)
+         children.push_back(reinterpret_cast<Node *>(child->_private));
+   }
+   while (child = child->next);
+   return children;
 }
 
 const Node::NodeList Node::get_children(const std::string& name) const
@@ -95,12 +87,12 @@
   return static_cast<Element*>(add_child(tmp));
 }
 
-Node* Node::add_child(Node* node) {
-  if(!node)
-    return NULL;
-
-  _children.insert(_children.end(), node);
-  return node;
+Node* Node::add_child(Node* node)
+{
+   if(!node) return 0;
+   xmlNode *result = xmlAddChild(_impl, node->_impl);
+   
+  return reinterpret_cast<Node *>(result->_private);
 }
 
 TextNode* Node::add_content(const std::string& content)
@@ -112,21 +104,17 @@
 
 void Node::remove_child(Node* node)
 {
-  if(node)
-  {
-    _children.remove(node);
-    delete node;
-  }
+   delete node;
 }
 
 std::string Node::get_name() const
 {
-  return _name;
+  return (char *)_impl->name;
 }
 
 int Node::get_line() const
 {
-  return _line;
+   return XML_GET_LINE(_impl);
 }
 
 bool Node::has_content() const
@@ -160,6 +148,50 @@
   }
 }
 
+void Node::set_child_content(const std::string& content)
+{
+  TextNode* node = get_child_content();
+  if(!node)
+    node = add_content(content);
+  else
+    node->set_content(content);
+}
+
+Node::AttributeList Node::get_attributes() const
+{
+  AttributeList attributes;
+  for (xmlAttr *attr = _impl->properties; attr; attr = attr->next)
+  {
+    attributes.push_back(reinterpret_cast<Attribute *>(attr->_private));
+  }
+  return attributes;
+}
+
+Attribute *Node::get_attribute(const std::string &name) const
+{
+  for (xmlAttr *attr = _impl->properties; attr; attr = attr->next)
+  {
+    if (attr->name == (xmlChar *)name.c_str())
+      return reinterpret_cast<Attribute *>(attr->_private);
+  }
+  return 0;
+}
+
+Attribute *Node::set_attribute(const std::string &name,
+                               const std::string &value)
+{
+  xmlAttr *attr = xmlSetProp(_impl,
+			     (xmlChar *)name.c_str(),
+			     (xmlChar *)value.c_str());
+  if (!attr->_private) return new Attribute(attr);
+  else return reinterpret_cast<Attribute *>(attr->_private);
+}
+
+void Node::remove_attribute(const std::string &name)
+{
+  xmlUnsetProp(_impl, (xmlChar *)name.c_str());
+}
+
 void Node::write(xmlDocPtr doc, xmlNodePtr parent) const
 { 
   xmlNodePtr node = 0;
@@ -172,16 +204,16 @@
 
     node = xmlNewChild(parent, NULL /* ns */, (xmlChar*) get_name().c_str(), 0);
     if(!node)
-      throw internal_error("xmlNewChild() returned NULL");
+       throw internal_error("xmlNewChild() returned NULL");
   }
 
   if(node)
   {
-    //Add the child nodes:
-    for(NodeList::const_iterator iter = _children.begin();
-       iter != _children.end();
-       ++iter)
-    {
+     NodeList children = get_children();
+     for(NodeList::const_iterator iter = children.begin();
+         iter != children.end();
+         ++iter)
+     {
       //Add the nodes to this parent node:
 
       //write() is a virtual function, implemented differently for each node type.
@@ -189,15 +221,6 @@
     }
   }
 
-}
-
-void Node::set_child_content(const std::string& content)
-{
-  TextNode* node = get_child_content();
-  if(!node)
-    node = add_content(content);
-  else
-    node->set_content(content);
 }
 
 } // namespace xmlpp
Index: libxml++/nodes/node.h
===================================================================
RCS file: /cvsroot/libxmlplusplus/libxml++/libxml++/nodes/node.h,v
retrieving revision 1.5
diff -u -r1.5 node.h
--- libxml++/nodes/node.h	25 Dec 2002 01:25:07 -0000	1.5
+++ libxml++/nodes/node.h	29 Jan 2003 03:33:52 -0000
@@ -13,6 +13,8 @@
 
 #include <libxml/tree.h>
 
+#include <libxml++/attribute.h>
+
 namespace xmlpp {
 
 class TextNode;
@@ -25,6 +27,8 @@
 {
 public:
   typedef std::list<Node*> NodeList;
+  typedef std::list<Attribute*> AttributeList;
+  typedef std::map<std::string, Attribute*> AttributeMap;
 
   explicit Node(const std::string& name);
   explicit Node(const Node* from);
@@ -56,11 +60,14 @@
   virtual void write(xmlDocPtr doc, xmlNodePtr parent = 0) const;
 
 protected:
-  Node::NodeList _children;
-  
+  AttributeList get_attributes() const;
+  // FIXME: the following only returns explicitely provided
+  // attributes, not default ones declared in the dtd.
+  Attribute *get_attribute(const std::string &) const;
+  Attribute *set_attribute(const std::string &, const std::string &);
+  void remove_attribute(const std::string &);
 private:
-  std::string _name;
-  int _line;
+   xmlNode *_impl;
 };
 
 } // namespace xmlpp