[PropertyTree] Rewrite - Please Review

Hi PropertyTree users,
I have completed the main bulk of the property tree rewrite/cleanup. In
the course of this, I have made the code standard-compliant (it
previously used a std::list of an incomplete type, which is not,
strictly speaking, valid), made it follow the claimed complexities
(find() was O(n), although the docs said it's O(log n)), and revised the
customization interface quite thoroughly.
Inevitably, there were breaking changes in this process.
I would ask everyone who has existing code using PropertyTree to look at
these changes and tell me their best guess at how much effort it would
be to update the code to conform with the new interface, and how I could
make this easier.
The development is still in a branch, so you need to check out the SVN
branch
http://svn.boost.org/svn/boost/branches/sredl_2009_05_proptree_update to
try out the new version.
The breaking changes are as follows:
1) Probably the most common change need: ptree::find() now returns the
new type assoc_iterator (const_assoc_iterator for the const version)
instead of iterator (const_iterator). This means that you need to change
the type of the variable you capture the result with to assoc_iterator.
It also means you need to compare the result of find() against
ptree::not_found() instead of ptree::end(). You can get an iterator from
an assoc_iterator by passing it to ptree::to_iterator(). The other
projection direction is currently not supported, although it would be
trivial to add.
2) The number and order of template arguments to basic_ptree has been
changed. The old arguments were
* Key comparison predicate
* Key type
* Path type
* Data type
* Translator type
The new arguments are
* Key type
* Data type
* Key comparison predicate (defaults to std::less<Key>)
The old order prevented defaulting and wasn't very useful in general.
The new order is consistent with std::map. The path and translator
arguments have been removed to simplify the interface of basic_ptree.
Equivalent functionality is available (see below).
If you used your own instantiation of basic_ptree before, instead of
simply using the (wi)ptree typedefs, you will have to adapt your code to
the new order. If you used custom path and/or translator types, read on.
3) put and put_child previously had a final parameter 'bool
do_not_replace'. I don't like boolean parameters, especially those whose
name contains a negation. I removed the parameter, and replaced its
functionality by a pair of new functions, add and add_child. If you
previously called put or put_child passing true for do_not_replace, you
must change these calls to add or add_child, respectively. If you ever
passed false explicitly, you have to remove this parameter.
4) Using custom path types has changed. You can no longer create
individual basic_ptree instantiations with custom path types. Instead, a
key type has a fixed path type associated with it. I can bring this
functionality back pretty easily if required, but I want to see use
cases for this first. (Such things are why I make this call for
reviews.) Less template parameters to ptree is good, in my opinion.
To use a key type other than std::basic_string specializations, you have
to specialize boost::property_tree::path_of for this key type. The
path_of struct contains a single type called type, which is the path to
use for this key type.
The way paths are implemented has also changed thoroughly. Previously,
you had to implement a const and a non-const variant of get_child, plus
put_child (with do_not_replace making that effectively two functions
too). These functions contained a lot of logic that really belonged into
ptree, making implementing custom paths tedious and error-prone.
The new interface requires you to implement four functions, but they're
far simpler. The functions are described in ptree_fwd.hpp in C++0x
concept syntax, but here's the gist:
bool path::empty() const -> is the path empty?
bool path::single() const -> does the path contain a single element?
key_type path::reduce() -> chop off the first part of this path and
return it (basically a pop_front that returns the value).
std::string dump() const -> get a readable form of this path for
exception messages; feel free to return "" here if you don't care.
The new interface should be easy to implement, and the tree traversal is
now where it belongs (in the internal implementation functions walk_path
and force_path).
5) Using custom translators has changed. A ptree no longer has a fixed
translator associated with it. Instead, you can supply a custom
translator to every get_value/get/put_value/put operation or use the
default, which depends on the tree's data type and the type you
extract/insert. An example:
ptree pt;
pt.put("k1.k2", 100); // Uses the default translator for std::string+int.
// Note: the explicit template parameter here will go away.
float f = pt.get<float>("k1.k2", my_special_float_string_converter());
The tree's own data type is called the internal type, while the
parameter/return type is called the external type. The default
translator for a internal+external pair is set by specializing the
boost::property_tree::translator_between struct. By default,
PropertyTree supplies an id_translator for when the internal and
external types are the same, and a stream_translator for converting
between a basic_string and anything else. Together, they faithfully
emulate the behavior of the old default translator.
Writing a custom translator is easy. It's a class that provides two
typedefs and two functions:
typename internal_type -> the internal data type
typename external_type -> the external data type
optional

Sebastian Redl skrev:
4) Using custom path types has changed. You can no longer create individual basic_ptree instantiations with custom path types. Instead, a key type has a fixed path type associated with it. I can bring this functionality back pretty easily if required, but I want to see use cases for this first. (Such things are why I make this call for reviews.)
May I suggest that you read the old review and review summary to see why this was added? I guess this goes for all changes. -Thorsten

Sebastian Redl
I have completed the main bulk of the property tree rewrite/cleanup.
[snip]
Inevitably, there were breaking changes in this process.
I would ask everyone who has existing code using PropertyTree to look at these changes and tell me their best guess at how much effort it would be to update the code to conform with the new interface, and how I could make this easier.
Hi Sebastian, Your changes look good overall, thanks for doing them. I took a look at the effect on my code given your breaking changes. Making the required modifications should not be a problem at all. I'm a little concerned about binding the path type to the key type, but I don't have any real-life use-case to argue against this. I like your use of multi-index container, although perhaps you could use the BOOST_MULTI_INDEX_MEMBER macro in the index specification -- though the added portability may not matter given the other code. I can't think of any need to project from the regular iterator to the associative iterator. Be sure to briefly mention in the docs how the associative iterator iterates over its values versus that of the sequential iterator. Would it be worthwhile to provide an assoc_begin()? The translator changes seem good to me. -Ryan

On Fri, 12 Jun 2009 17:37:37 +0200, Sebastian Redl
[...]I would ask everyone who has existing code using PropertyTree to look at these changes and tell me their best guess at how much effort it would be to update the code to conform with the new interface, and how I could make this easier.
There is a problem with the Spirit XML parser. When I define BOOST_PROPERTY_TREE_XML_PARSER_SPIRIT I get a compiler error: boost-1_39\boost\property_tree\detail\xml_parser_read_spirit.hpp(103) : error C2065: 'empty_ptree' : undeclared identifier If I use the Rapid XML parser the code compiles. I can't parse a UTF-8 encoded file correctly though. Is this a known problem? When I use boost::property_tree::wptree to parse a XML file with <test>ä</test> and UTF-8 encoding get() returns ä. The problem seems to be in read_xml_internal(). But as I couldn't test the Spirit XML parser this is maybe a known limitation of the Rapid XML parser? Boris
[...]

On Sat, 13 Jun 2009 18:11:29 +0200, "Boris Schaeling"
On Fri, 12 Jun 2009 17:37:37 +0200, Sebastian Redl
wrote: [...]I would ask everyone who has existing code using PropertyTree to look at these changes and tell me their best guess at how much effort it would be to update the code to conform with the new interface, and how I could make this easier.
There is a problem with the Spirit XML parser. When I define BOOST_PROPERTY_TREE_XML_PARSER_SPIRIT I get a compiler error:
boost-1_39\boost\property_tree\detail\xml_parser_read_spirit.hpp(103) : error C2065: 'empty_ptree' : undeclared identifier
If I use the Rapid XML parser the code compiles. I can't parse a UTF-8 encoded file correctly though. Is this a known problem?
I know I haven't tested any XML parser other than the RapidXML one, and I know I haven't tested it with non-ASCII input. Insofar it's a "known" problem, yeah. :-) Thanks for telling me. I actually plan to drop all XML parsers save one. There's no reason to maintain redundant code. Sebastian

On Sat, 13 Jun 2009 18:37:08 +0200, Sebastian Redl
On Sat, 13 Jun 2009 18:11:29 +0200, "Boris Schaeling"
wrote: On Fri, 12 Jun 2009 17:37:37 +0200, Sebastian Redl
wrote: [...]I would ask everyone who has existing code using PropertyTree to look at these changes and tell me their best guess at how much effort it would be to update the code to conform with the new interface, and how I could make this easier.
There is a problem with the Spirit XML parser. When I define BOOST_PROPERTY_TREE_XML_PARSER_SPIRIT I get a compiler error:
boost-1_39\boost\property_tree\detail\xml_parser_read_spirit.hpp(103) : error C2065: 'empty_ptree' : undeclared identifier
If I use the Rapid XML parser the code compiles. I can't parse a UTF-8 encoded file correctly though. Is this a known problem?
I know I haven't tested any XML parser other than the RapidXML one, and I know I haven't tested it with non-ASCII input. Insofar it's a "known" problem, yeah. :-) Thanks for telling me.
I can't remember if I had a problem with UTF-8 files before. I know though that I always used the Spirit XML parser. If you have an idea what I should change to get rid of the compiler error I can give Spirit a try.
I actually plan to drop all XML parsers save one. There's no reason to maintain redundant code.
No problem with this (as long as it won't be required to setup another external library to use the XML parser). Boris
participants (4)
-
Boris Schaeling
-
Ryan Gallagher
-
Sebastian Redl
-
Thorsten Ottosen