[boost] Re: review request: assignment library

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2003-09-17 04:05:03

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++.
_______________________________________________________________
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.
_______________________________________________________________
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<>
     ...
_______________________________________________________________
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.
_______________________________________________________________
5. examples at the beginning of index.html:

   The
#include <vector>
   there is redundant.

Second example with map<> includes <vector> instead of <map>.
_______________________________________________________________
6. Maybe the namespace can be shortened from "assignment" to "assign".
_______________________________________________________________
7. index.html, "Reference" section: there should be only one font used in
first line.
_______________________________________________________________
7. index.html, "Reference" section: the first example: can you add
   inline comments to the code?

   Also elsewhere.
_______________________________________________________________
8. index.html: for aesthetic reasons, <hr> can be put between sections
   to improve readability (I like them, you can see ;-)
_______________________________________________________________
9. Typo (missing dot):

#include <boost/assign/fixed_size_assignerhpp>
_______________________________________________________________
10. index.html, uBlas example: there is "try" without catch.
_______________________________________________________________
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.
_______________________________________________________________
12. index.html, portability section|: the limitations should be detailed
    - which compilers and what.
_______________________________________________________________
13. would it be possible to support mode with disabled exception handling?
    (indicated by BOOST_NO_EXCEPTIONS)
_______________________________________________________________
14. array.hpp: can

#include "../fixed_size_assigner.hpp"

be changed to

#include <boost/...>

The file array.hpp is also void.

Dtto for mult_array.hpp.
_______________________________________________________________
15. property_map.hpp has 0 size.

_______________________________________________________________
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.
_______________________________________________________________
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).
_______________________________________________________________
18. Support for (coming to review) circular_buffer<> library may be added.
_______________________________________________________________
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.
_______________________________________________________________
20. Question: do usual rules of automatic conversion apply in this library?
    I didn't find in documentation.
_______________________________________________________________
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.)
_______________________________________________________________
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.
_______________________________________________________________
EOF