From: Gennadiy Rozental (gennadiy.rozental_at_[hidden])
Date: 2006-04-19 22:39:18
I am really busy lately and won't have time for complete review. But since I
am intimately familiar with this domain, I've decided to take peek on my way
First and foremost I would like to remind everybody that we already have
one library intended to cover this problem domain (completely unacceptable
IMO - but that's another story). This library not only do not address issues
of existent solution it's not even comparatively close feature wise. It does
present some additional media formats. But this could as well be done as a
add-on to existing solution.
So my vote is NO, thank you very much.
Here some notes in no particular order from my skimming through docs and
1. Name is really bad. Originally I thought this submission has something to
do with property_map and was surprised to see what I see.
2. Why would you need fast lookup? In 99.9% of cases each variable is
accessed single time in a program lifetime. No need to optimize so that you
program startup time go from 5 mls to 5 mks. You won't notice this anyway
(And in reality it's not even that big difference. Most of your startup time
you will spend initiating either network connection or some GUI component or
3. Even if you insist on alternative lookup. Why not just use single typedef
with multi_index instead of all the implementation.
4. Extra functionality (to the one in multi_index) should go in free
functions not in member functions - it would enhance encapsulation and
5. CLA parser is a joke. It's unacceptable any way you present it.
6. I personally believe that inline implementation in this case is actually
is rather a disadvantage. I would be perfectly happy to stick to ASCII for
configuration purposes. Even if you insist on wide char support, I would
implement it as a thin template wrapper around offline implementation that
operates with void*. Plus some runtime polymorphism could be used.
7. ptree_utils reinvent the wheel. Why not use Boost string algorithms?
8. I keep repeating: Traits could not be template parameters. You should had
have three template parameter KeyType, ValueType and ComparePolicy. The same
way as std::map is doing
9. Access interface are lacking. If I got string identify subsystem and
string that identify parameter separately why do I need to concatenate them
to get to the value
10. General note: the whole design/implementation is unnecessary
complicated. You general part is about 50k. In Boost.Test utils section I
have a component with very similar rationale but better design (IMO
obviously) implemented in 16k.
11. Generating part if the design/implementation is completely unnecessary.
It may? be useful in 1% of the usage cases and in those cases I would stick
to some alternatives.
IMO library presenters at best needed to do some comparative analysis with
existing solution and present it as a part of the review.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk