Re: [boost] [Boost Review] Property Tree Library

From: Sebastian Redl (sebastian.redl_at_[hidden])
Date: 2006-04-19 06:38:06

Here's my preliminary review of the property tree library.

>      * Are you knowledgeable about the problem domain?
>  
>
I'm a student of software engineering and have repeatedly written 
programs that require external configuration, mostly in C++, Java and PHP.

>      * How much effort did you put into your evaluation?
>        A glance? A quick reading? In-depth study?
>  
>
I've begun using the library as the main means of configuration in a 
project. I have not yet, however, compiled any code or looked at the 
library source code.

>      * What is your evaluation of the design?
>  
>
The design is straightforward and sound. The compatibility with standard 
containers makes the learning curve shallow, and the specialized 
functions offer an easy interface.

>      * What is your evaluation of the implementation?
>  
>
I did not look at the implementation.

>      * What is your evaluation of the documentation?
>  
>
The documentation is generally good. However, it could be better 
structured - in particular, the documentation index page is too long and 
needs to be split into parts. The documentation also requires attention 
from a native English speaker to correct the various grammar errors.

>      * What is your evaluation of the potential usefulness
>        of the library?
>  
>
High. It offers a flexible and universal interface to many different 
forms of configuration sources in a way that I have yet to see in any 
other language/library.

>      * Did you try to use the library? With what compiler?
>        Did you have any problems?
>  
>
I've so far written a bit of source code using the library, but have not 
yet compiled the code. It will eventually be compiled using GCC 3.4.x 
and 4.x.

>And finally, every review should answer this question:
>
>      * Do you think the library should be accepted as a Boost library?
>        Be sure to say this explicitly so that your other comments
>        don't obscure your overall opinion.
>  
>
Yes. Already it transforms a job I've been dreading to do (and so far 
have reinvented every time, because I was never satisfied with my 
approach) into a matter of 5 lines. (Literally!) Enhancements from here 
on are incremental and their lack does not matter right now.

>In particular, consider if you can answer the following questions:
>
>- was the library suitable for your daily xml-tasks? If not, what was 
>missing?
>  
>
The library seems very well suited to read and save XML configuration 
files. I would suggest, however, that versions of read/write_xml are 
introduced that allow to specify the special keys for attributes, 
comments and text. Also, a flag should be added that specifies 
whitespace handling. (Ignore, preserve, collapse.)

>- was the library's performance good enough? If not, can you suggest 
>improvements?
>  
>
No performance testing done.

>- was the library's design flexible enough? If not, how would you 
>suggest it should be redesigned to broaden its scope of use?
>  
>
Seems flexible enough for me, thanks to the key and value type traits.

Additional suggestions:
1) Having a separate namespace for the interface of each parser seems to 
be overly verbose. The property_tree namespace is sparsely populated 
(just ptree, a few predefined traits classes, and the exceptions if I'm 
not mistaken), so the read_* and write_* functions could reside there. 
Alternatively, a single namespace parsers could be created to hold all 
interface functions. Assuming pt is an alias for the property tree 
namespace, pt::xml_parser::read_xml contains redundant information about 
the type being read.
2) Add a parser for Java-like .properties files. The hierarchy could be 
built using common prefixes, i.e.
build.dirs.libraries = /usr/lib
build.dirs.headers = /usr/include
could be transformed to
build {
  dirs {
    libraries = /usr/lib
    headers = /usr/lib
  }
}
3) The default separator should be settable. E.g. if I want to always 
separate my paths with '/' (I could be porting from a different 
library), I want to be able to write:
pt.set_default_separator('/');
string libs = pt.get("build/dirs/libraries");

Perhaps I'll have additional comments at a later point.

Sebastian Redl