[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<external_type> Tr::get_value(internal_type) -> convert from the internal to the external type, boost::none on failure optional<internal_type> Tr::put_value(external_type) -> convert from the external to the internal type, boost::none on failure That's it for the changes. The new version of property tree is not quite complete yet. I still need to polish the documentation a little and document the new extension mechanisms properly. I also need to clean up a little. Finally, the INI parser doesn't work at the moment. However, the main functionality (tested by property_tree_test) plus the XML, JSON and INFO parsers should all work now. Looking forward to your feedback. Sebastian
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 <sebastian.redl <at> getdesigned.at> writes:
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 <sebastian.redl@getdesigned.at> 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? 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" <boris@highscore.de> wrote:
On Fri, 12 Jun 2009 17:37:37 +0200, Sebastian Redl <sebastian.redl@getdesigned.at> 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 <sebastian.redl@getdesigned.at> wrote:
On Sat, 13 Jun 2009 18:11:29 +0200, "Boris Schaeling" <boris@highscore.de> wrote:
On Fri, 12 Jun 2009 17:37:37 +0200, Sebastian Redl <sebastian.redl@getdesigned.at> 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