[boost] [Boost Review] Property tree review

From: Jeff Hardy (jdhardy_at_[hidden])
Date: 2006-04-29 14:17:17

I'm going to vote YES on inclusion of the Property Tree library. It
has its faults, but I think it's useful enough right now to be part of
Boost.

* What is your evaluation of the design?
The most valuable parts of the library are the parsers and the path
functions. I agree with what others have mentioned: the paths should
be seperate from the tree.
I would be using it to parse various file formats into a tree, and
then use my own path language on top of that.

For me, then, the parsers are the most important part of this library.
Having a (mostly) common structure is very nice. The reading and
writing interface is as simple as possible; navigating the tree is
also resonably simple.

* What is your evaluation of the implementation?
I didn't inspect the implementation closely, but I did notice some
things during use.

I don't like how XML attributes are stored. It should at least be an
option to treat <x y="1" z="2" /> and
<x>
        <y>1</y>
        <z>2</z>
</x>
as the same structure, without the "<xmlattrs>" node. If there's a
confilict between an attribute name and a child element name, then
reading/writing should fail.

The ability to preserve comments in a file would be very nice.

* What is your evaluation of the documentation?
A bit sparse. The reference was mostly what I used, but more examples
in the docs is always a good idea. A better "mission statement" about
what the library is for would help as well; right now, it's not very
clear what the author thinks the library is trying to do.

The tree descriptions used in the descriptions of the parsers are not
very clear; specifying what is in child.second.data() and what is in
child.first should be explicit.

An example of directly manipulating the tree (without using
get()/put()) would be useful as well.

* What is your evaluation of the potential usefulness of the library?
As a general XML parser, it's not very useful (no namespaces, etc.);
as a generic tree, it's not very useful (not general enough). As a
utility for parsing various file formats into similar data structures
(and writing them back), it's useful.

Loading program configuration seems to be the primary goal, and it
does that well enough. Trying to get the library to do other things
might be stretching it too far.

* Did you try to use the library? With what compiler? Did you have any problems?
Yes, with MSVC 8. I had no problems beyond my own reading incomprehension.

* How much effort did you put into your evaluation?
  A glance? A quick reading? In-depth study?
I adapted the backend of my own configuration system (which previously
used TinyXML) to use property trees. However, I didn't use the get*()
functions, since I have my own path system already built.

* Are you knowledgeable about the problem domain?
Resonably.

I plan on using the library whether it is accepted into Boost or not,
and I'd like to thank Marcin for his work.

-Jeff Hardy