[boost] Re: review request: assignment library

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2003-09-17 10:59:27

Thanks for you comments.

"Pavel Vozenilek" <pavel_vozenilek_at_[hidden]> wrote in message
news:bk988e$7qo$1_at_sea.gmane.org...
> Hello Thorsten,
>
> I looked briefly on your library and have few notes (bellow).
>
> /Pavel
> _______________________________________________________________
> 1. Can you add
>
> #if _MSC_VER >= 1200
> #  pragma once
> #endif
>
> to the assign.hpp and others? This is helpful for MSVC and Intel C++.

yep. I'll use Peter's version.

> 2. assign.hpp:
>
> It may be better to use
>
> #include <boost/assign/stl.hpp>
>
> than
>
> #include "assign/stl.hpp"
>
> The rules for file lookup can vary between compilers.

done.

> 3. assign.hpp brings in the whole STL and uBlas. This would have
>    impact on compilation time.
>
>    It would be useful to have also finer granularity headers,
>    something like Serialization library does.
>
>    Say:
>      - assign.hpp - everything
>      - assign/stl.hpp - STL
>      - assign/vector.hpp - only vector<>
>      ...

yep, I agree. notice that most of STL is not needed when the compiler
support ordering of function templates.

> _______________________________________________________________
> 4. index.html: the Bjarne's joke doesn't seem well placed - I suspect
>    it was told long ago when very few people had clue. He likely
>    changed his opinion since.
> _______________________________________________________________

Perhaps. I'm not trying to make fun of him, though. Do other people thinks
such a comment is misplaced?

> 5. examples at the beginning of index.html:
>
>    The
> #include <vector>
>    there is redundant.

yes it is if the compiler supports ordering of function templates.

> Second example with map<> includes <vector> instead of <map>.
> _______________________________________________________________
> 6. Maybe the namespace can be shortened from "assignment" to "assign".

I don't think so. According to [7.3.2.4]
"A namespacename or namespacealias shall not be declared as the name of any
other entity in the same declarative region."

so we can't have a namespace 'assign' and a function 'assign' inside that
namespace. Not all compilers seem toobey this.

> _______________________________________________________________
> 7. index.html, "Reference" section: there should be only one font used in
> first line.
> ______

I'm not sure what you mean here.

> _______________________________________________________________
> 11. index.html: maybe the reference section can be made more
>     'technical'.
>
>     I mean e.g. table of functions, with effects, constrainst, exceptions
> etc.
>
>     It reads better (to some) than the 'literal' way with full sentences.

Before I do so I would like to know if other people think it is really
necessary. I mean, one should very quikly realize how thinks works and then
don't think more about about it. For example, you easily see that you can
use operator+= () and the comma and really donøt need more information. If
you need the extra info, I think it is mostly there.

> _______________________________________________________________
> 12. index.html, portability section|: the limitations should be detailed
>     - which compilers and what.

I don't know exactly yet since I haven't run the latest version on all the
compiers. I just put the text there as a place holder.

 _______________________________________________________________
> 13. would it be possible to support mode with disabled exception handling?
>     (indicated by BOOST_NO_EXCEPTIONS)

yep, and I have done so.

> _______________________________________________________________
> 15. property_map.hpp has 0 size.

its not really intended to be used yet :-)

> _______________________________________________________________
> 16. fixed_size_assigner.hpp:
>
>    destructor throws!!!! This is very, very, very dangerous.
>    If it is somehow safe due to uncaught_exception, it should be
>    in detail explained there.

I guess I have missed something here. Won't uncaught_exception() guard my
use of throwing exceptions?

> _______________________________________________________________
> 17. The library can consider boost::call_traits<T>::param_value.
>
>     This way primitive types are be passed by value, instead of by
> reference.
>
>     Newest STLport (in beta) was able to gain 5-10% speedup for
>     vector<>::push_back(int).

I didn't do it because some small benchmarks I did some time ago showed no
difference. So I though it would be better to improve compile time. But I
take you word for it, so I'll update the code acordingly..

> _______________________________________________________________
> 18. Support for (coming to review) circular_buffer<> library may be added.

As the documentation states, it should work "out of the box" since it is a
standard complient container.

> _______________________________________________________________
> 19. Wish feature: would it be somehow possible to add 'dynamic
> initialisation'?
>
> I mean something like:
>
> vector<int> values;
> values += 10, 20, variable1, 40, foo();
>
> where the library would copy current value of 'variable1' and
> result of calling function foo() into container.
>
> I have no clue if that's feasible or within intentions of the library.

I thought it was obvious that this can be done already :-) I'll remove the
word "constant" from the introduction.

> _______________________________________________________________
> 20. Question: do usual rules of automatic conversion apply in this
library?
>     I didn't find in documentation.

Yes. I've added a small paragraph to the reference section.

> _______________________________________________________________
> 21. Are the containers (if possible) resized prior to the given amount of
> data?
>      (I believe the do not and it cannot be done but better to ask.)

assign() does a .clear() first So it cannot be used with container adapters.

> _______________________________________________________________
> 22. Support for slist, multimap, multiset, hash_map, hash_set,
> hash_multimap, hash_multiset
>       may be also added (if feasible).
>
>      Maybe also boost::tuple.
> _______________________________________________________________
> 23. Can you add support for std::pair? I know there's make_pair() but for
> consistency.
> _______________________________________________________________

multimap/multiset are already supported. the hash_stuff and slist works "out
of the box" but  are not supported for compilers without partial template
ordering.

there is a  lot of more stuff that can be given support, but I just wanted
to get the basics right first. It should be quite trivial to add extra
support though; I'll look into it soon.

best regards

Thorsten