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

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2006-04-20 10:19:54

Hello Marcin,

> First of all, from some of your comments I see you must have looked at the 
> older revision. Rev.5 does not have any pointers in the interface, and no 
> longer has get_d, get_b variants.
>

Gosh, I found I have two source code trees here
and I picked up the old one. Sorry for the mess.

I recall everything I said and will review the latest
version again.

 _____________________________________________________
>> 1. docs: it is rather unusual for me read code
>>    without syntax highlighting.
>
>
> The problem is, it is written and maintained in HTML. I could add code 
> coloring, but that would render it close to unmaintainable. Any change 
> would be a nightmare. I should have used QuickBook, but unfortunately I 
> had problems setting up the toolchain. So I stuck to HTML. Btw. some other 
> well estabilished libraries in boost also do not have code coloring, and I 
> didn't notice anybody complaining.
>
Newer libraries generaly have syntax highlighting.
I complain because it is years and years since I wrote code
without colorizing editor.

 _____________________________________________________
>> 6. Possible feature:
>>
>> "...there is an additional indexing data structure
>> inside property tree that allows fast searches..."
>>
>> There may be a type trait that disables creating
>> of such structure, where the search complexity
>> would degrade gracefuly to O(N).
>
>
> I know, you said that before, I have it hovering on my list of things to 
> be done. It has never got big enough priority though.
>

It seems I rerun my previous review once again.
I do not keep the previous texts so it may happen
more than once I "discover" something repeatedly.

 _____________________________________________________
>> 7. The temptation to use std::lower_bound and similarly
>>    dangeroud std algorithm may be eliminated by defining
>>    unimplemented specialisations for ptree
>
>
> You can always sort the tree and then use the algorithm safely, so banning 
> it would not be wise.
>

Hmm. Perhaps specialisations could be written that in debug
mode check the tree is actually sorted.

 _____________________________________________________
>> 15. The headers may have
>>
>> #if (defined _MSC_VER) && (_MSC_VER >= 1200)
>> #  pragma once
>> #endif
>

> Btw. is compilation time really shortened that much? I think that it only 
> removes the preprocessing step, which is done in a snap anyway. I haven't 
> done any measurements so I might be wrong.
>

Small but noticeable if applied rigorously:
http://lists.boost.org/Archives/boost/2003/09/52773.php

 _____________________________________________________
>> 25. I tried to compile the test with BCB

> That's great. If somebody else with better knowledge of BCB intricacies 
> could do it, it would be nice. On the other hand I'm afraid of introducing 
> too many maintenance problems (aka ifdefs), or compromising the 
> design/performance of the library to make old stuff happy.
>

#ifdefs will be necessary for BCB.

Generally, most of Boost libraries are littered with #ifdefs
to be almost unreadable. A "clean" version of Boost has been
suggested but it is not likely to happen in next few years.

/Pavel