[Boost Review] Property Tree Library
I'm voting NO on "property tree". I'm not persuaded that this library should be added to boost. Here is my review: * What is your evaluation of the design? Interesting. The "Property Tree" library attempts to meet a real need that I have had. The design appears to be appears to be adequate at first, but the more time I spent with working through the examples, the less I liked it. * What is your evaluation of the implementation? I think the implementation is pretty good. Overall, the author(s) have utilized modern c++ and established boost's "best-practices", including using boost::spirit for the "xml" parser. I'm not sure why he didn't use boost::spirit or boost::xpressive for the other parsers though. * What is your evaluation of the documentation? Inadequate, at least compared to the better documented boost libraries. More work is need here. * What is your evaluation of the potential usefulness of the library? Each individual part of the library is very useful. * Did you try to use the library? With what compiler? Did you have any problems? gcc. No problems. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I studied documentation and the source code for a couple of hours. There is variety of interesting stuff hidden in this library. I ran the examples and created a couple of my own. * Are you knowledgeable about the problem domain? Yes * Do you think the library should be accepted as a Boost library? No I'm voting NO on "property tree". I'm not persuaded that this library should be added to boost. It should be broken up into seperate boost::spirit or boost::xpressive grammers and submitted seperately. Grouping all this stuff under the "property tree" library is convenient, but not up to the standards of a boost library.
Tom Brinkman wrote:
I'm voting NO on "property tree". I'm not persuaded that this library should be added to boost. Here is my review:
* What is your evaluation of the design?
Interesting. The "Property Tree" library attempts to meet a real need that I have had.
The design appears to be appears to be adequate at first, but the more time I spent with working through the examples, the less I liked it.
What did you not like and why? Thanks -Thorsten
The design appears to be appears to be adequate at first, but the more time I spent with working through the examples, the less I liked it.
Please could you state specific things you didn't like.
I think the implementation is pretty good. Overall, the author(s) have utilized modern c++ and established boost's "best-practices", including using boost::spirit for the "xml" parser. I'm not sure why he didn't use boost::spirit or boost::xpressive for the other parsers though.
Here's the current implementation status of various parsers: XML - Spirit JSON - Spirit INI - not using Spirit INFO - not using Spirit Registry - irrelevant Cmdline - irrelevant The reason I didn't use Spirit to parse INI file is its simplicty. If you look in ini_parser.hpp, read function fits on one screen. This will not be considerably simpler with Spirit, it might even be longer if you consider that you'll need to define grammar, function objects for actions. I agree it would be more maintainable with Spirit. INFO file is another story, I implemented first parsers for it long time ago, even before I started using boost. I once rewrote in it Spirit, but that - for some reason (maybe my incompetence at that time) - made parser significantly slower. So I returned to original version. Because it works fine as it is now, I see little reason to rewrite it. For those who may be interested, in examples dir there is an example which implements INFO with Spirit. It is intended to be a grammar specification (a nice side effect of Spirit is that it is a compilable and runnable grammar specification :-)
* What is your evaluation of the documentation?
Inadequate, at least compared to the better documented boost libraries. More work is need here.
Please specify what would you like expanded/added. I'm already aware that there is precious little docs on customization/traits part of the library. Best regards, Marcin
Marcin Kalicinski wrote:
The design appears to be appears to be adequate at first, but the more time I spent with working through the examples, the less I liked it.
Please could you state specific things you didn't like.
I think the implementation is pretty good. Overall, the author(s) have utilized modern c++ and established boost's "best-practices", including using boost::spirit for the "xml" parser. I'm not sure why he didn't use boost::spirit or boost::xpressive for the other parsers though.
Here's the current implementation status of various parsers:
XML - Spirit JSON - Spirit INI - not using Spirit INFO - not using Spirit Registry - irrelevant Cmdline - irrelevant
The reason I didn't use Spirit to parse INI file is its simplicty. If you look in ini_parser.hpp, read function fits on one screen. This will not be considerably simpler with Spirit, it might even be longer if you consider that you'll need to define grammar,
A explicit grammar<> is not necessarily required, you can construct the rules in place in the call to parse, or use functor parsers. The benefit of using spirit is that the grammar or rules can be easily compared with the ebnf for the file format and be determined to account for all valid .ini files. The comments could also then be preserved as well as PT nodes.
function objects for actions. I agree it would be more maintainable with Spirit.
The function object IMO should be part of the by the PT library 'parser' interface. This is the back_end interface when one wants to parse their own format and populate a PT. These would be common to all formats, and lend credence to the PT library being format agnostic. Jeff Flinn
participants (4)
-
Jeff Flinn -
Marcin Kalicinski -
Thorsten Ottosen -
Tom Brinkman