El 08/08/2011 23:48, Pierre Morcello escribió:
Hello Ion,
This is not a review.
I took 3 hours to read the docs, checks some tests, try to check some sources, but I am still not used to read so few comments in the code. I played a little with the library then, tried to encapsulate some std::auto_ptr<X> in the new containers and things like that.
Thanks for taking your valuable time ;-)
Based on that very little experience, I would like to share some thoughts and ask a few questions on the proposed boost.containers library.
Go ahead.
1/ I would really appreciate that the docs were more complete, at least for the technical part. => ex : "flat_map : more memory friendly and with faster searches" => In what way? is the algorithm complexity lesser than previous systems? or do you mean that contiguous memory is more cache-friendly? => do you provide different kernel implementations? => is there a paper on the subject? => Is there an evaluated algorithm complexity for the stable_vector? => is memory contiguous like a std::vector? can I use memcpy on several elements? These are just examples of questions that I expect to be answered by the docs.
Ok, this was already mentioned in previous reviews. I'll add new chapters for non-standard containers like flat_xxx, stable_vector and slist.
EDIT : I read proposition to embed elements of Joaquin M Lopez Munoz's blog. This is exactly that kind of informations I am looking for.
Ok
=> For the tree class, I can't remember any tree interface in the std. Could you point me to some informations ? Sorry If I miss something obvious.
No, there is not, it's just that all (ordered) associative containers were implemented as rbtrees.
2/ I was surprised (in a good way) to see some Andrei Alexandrescu's code in the library. Will the Loki licence be shipped now with boost? (This is not a problem for me).
Although originally I started with Loki code, current code is a complete rewrote (priv_insert_commit, etc. are concepts taken from Intrusive). It's just that I never changed the licencse like I did with SGI license note from vector. Nevertheless, the inspiration from Loki should be mentioned as the idea for a drop-in replacement for associative containers came from Loki::AssocVector.
3/ A few tests might be added. ex: Test the fact the vector usually are contiguous in memory. I can write this one if you are interested.
Patches welcome ;-)
4/ The 'move aware' containers are nice to use but as a drawback We are forced to use macro to define the types using the 'move' semantics.
We'll, it's the portable Boost.Move syntax. If you have a C++11 container you can directly use rvalue references. For C++03 code is much uglier to write than using macros :-(
What other container libraries can this work be related to?
Talking about portable containers, according libc++ (http://libcxx.llvm.org/), a high quality standard library requiring C++11 compiler, "STLport and the Apache libstdcxx library are two other popular candidates, but both lack C++'0x support. Our experience (and the experience of libstdc++ developers) is that adding support for C++0x (in particular rvalue references and move-only types) requires changes to almost every class and function, essentially amounting to a rewrite." "Further, both projects are apparently abandoned: STLport 5.2.1 was released in Oct'08, and STDCXX 4.2.1 in May'08." Boost.Container has some C++0x support and plans to support all C++11 features plus backport as much as possible (like we've already done with emplace, move semantics..) to C++03 compilers. For C++03 code STLport added some optimizations like "move semantic for STL containers combination like vector of vector", but I don't know if these optimizations are open to user-defined classes. list and other containers don't seem to support this.
There are some examples of containers requiring the value_type to support only the "defaut constructor + a swap() function". This is a concept I would really appreciate to see in boost.containers. In that case, I would very happily use this library. Otherwise I think I will stick with other containers classes.
Yes, default-contruct only types are supported in many operations, just like C++11 requires (say, resize(), emplace()...). I'll add this requirement to each function description.
5/ "The aim of the library is to offers advanced features" Is the proposed Boost.containers also an effort towards development of new features for containers? If that is the case, then what are the plans for the future ? Is there a roadmap?
At this moment, the goal is to fully support C++11 and backport as much as possible to C++03. After that, we can start experimenting new features (say, allocator improvements) or containers (just like stable_vector and flat_xxx family).
In my humble opinion, it would also be easier for exterior people to learn from and to contribute to your library if there were more comments in the code.
Most of code relies on Boost.Intrusive, but I'll add comments to code.
I congratulate you and the other authors for your work and efforts, and wish you the best for the review.
Thanks for your help! Ion