Re: [boost] API Review request: XML APIs for C++

From: Hamish Mackenzie (hamish_at_[hidden])
Date: 2003-06-12 15:44:23

Looks good.  I have only looked at the dom stuff so far.  Why are you
storing information in _private?  What goes in there that could not be
extracted when the nodes are accessed?  It seems like a lot of
unnecessary overhead.

Here are some suggestions...

1) parse_file could be

class parse_file
{
public:
  parse_file( const std::string & ) : {}
  ...
};

template<...>
class basic_document
{
public:
  basic_document( const parse_file & );
}

2) How about parse_string (based on xmlParseMemory)?

3) Why dom::basic_document::clone?  Why not have the copy constructor
and assignment operator should do a deep copy of the document?  This is
consistent with other containers. If you want to stick with clone return
an auto_ptr and and derive basic_document from boost::noncopyable.

4) How about something like

class basic_document
{
public:
  ...

  typedef node_iterator iterator;
  typedef const_node_iterator const_iterator;

  iterator begin()
    { return iterator( ptr_->children ); }
  const_iterator begin() const
    { return const_iterator( ptr_->children ); }
  iterator end() { return iterator(); }
  const_iterator end() const { return const_iterator(); }

  iterator root()
    { return iterator( xmlDocGetRootElement( ptr_ ) ); }
  const_iterator root() const
    { return const_iterator( xmlDocGetRootElement( ptr_ ) ); }
};

5) basic_xpath and basic_xpath_result should be derived from
boost::noncopyable

6) Leading _ chars are a no no (reserved for compiler implementors).  I
know _private is defined in libxml2, but what you see below is not...

$ grep ' _' *
basic_attribute.h:#ifndef _dom_basic_attribute_h
basic_attribute.h:#define _dom_basic_attribute_h
basic_comment.h:#ifndef _dom_basic_comment_h
basic_comment.h:#define _dom_basic_comment_h
basic_document.h:#ifndef _dom_basic_document_h
basic_document.h:#define _dom_basic_document_h
basic_dtd.h:#ifndef _dom_basic_dtd_h
basic_dtd.h:#define _dom_basic_dtd_h
basic_element.h:#ifndef _dom_basic_element_h
basic_element.h:#define _dom_basic_element_h
basic_node.h:#ifndef _dom_basic_node_h
basic_node.h:#define _dom_basic_node_h
basic_node.h:  basic_node_iterator(impl *current = 0) :
_current(current) {}
basic_node.h:  bool operator == (self i) { return _current ==
i._current;}
basic_node.h:  void increment() { _current = _current->next;}
basic_node.h:  void decrement() { _current = _current->prev;}
basic_node.h:  basic_node_const_iterator(const impl *current = 0) :
_current(current) {}
basic_node.h:  bool operator == (self i) { return _current ==
i._current;}
basic_node.h:  void increment() { _current = _current->next;}
basic_node.h:  void decrement() { _current = _current->prev;}
basic_pi.h:#ifndef _dom_basic_pi_h
basic_pi.h:#define _dom_basic_pi_h
basic_text.h:#ifndef _dom_basic_text_h
basic_text.h:#define _dom_basic_text_h
basic_traversal.h:#ifndef _dom_basic_traversal_h
basic_traversal.h:#define _dom_basic_traversal_h

Hamish

On Thu, 2003-06-12 at 03:45, Stefan Seefeld wrote: 
> hi there,
> 
> following some discussion we had some weeks ago,
> I'd like to invite everybody to review xml++.tgz at
> 
> http://groups.yahoo.com/group/boost/files/xml/
> 
> It's a DOM-like and a SAX-like API currently implemented
> on top of libxml2 (http://www.xmlsoft.org).
> 
> What it provides:
> 
> * parsing of xml files and creation of a document tree
> * manipulation of document tree, i.e. insertion and
>    deletion of nodes
> * node iteration, search (xpath based)
> * document output to a (xml) file
> 
> * event driven xml file parsing (sax)
> 
> To be added:
> 
> * validation (dtd, schema, etc.)
> * ?
> 
> Is there any interest in this library evolving
> into a boost::xml library ? If so, what needs to change,
> what needs to be added / removed ?
> 
> Regards,
> 		Stefan
> 
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

-- 
Hamish Mackenzie <hamish_at_[hidden]>