[boost] Re: [Review] Named parameters library

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2004-11-13 06:51:48

"Doug Gregor" wrote:

> The formal review of the Named Parameters library
> by David Abrahams and  Daniel Wallin starts today
>
I vote weak YES to accept this library.

- Its documentation is very weak and should
  be expanded *significantly*.
  More on it in note [8] bellow.

- There should be more tests.
- Problem with Intel C++ should be investigated.

/Pavel

_____________________________________________________
1. Headers should contain

#if (defined _MSC_VER) && (_MSC_VER >= 1200)
#  pragma hdrstop
#endif

on the beginning. It has effect on VC and
Intel C++ users.

_____________________________________________________
2. named_params.hpp:

Shouldn't the default for
  BOOST_NAMED_PARAMS_MAX_ARITY
be more than 5?

Why not >= 10.

_____________________________________________________
3. named_params.hpp:

Shouldn't the sub-namespace be named
"named_params_detail" instead of just "detail"?

Names like "nil" inside look quite collision prone.

_____________________________________________________
4. docs: the line

window* w = new_window("alert2", movability); // error!

should say "logical error" or "coding error" or something like that.

_____________________________________________________
5. docs: the chapter "Introduction" may be better
   named "Problem" but YMMV.

_____________________________________________________
6. could the code snippets be visually separated
   from the rest, e.g being in boxes and color syntax
   highlighted?

_____________________________________________________
7. Question: is there possibility BCB could be convinced
   to compile (part) if the library or is BCB
   completely without any chance?

_____________________________________________________
8. What I miss in documentation:

 - Information about overhead:
      - numbers/graphs for at least two compilers
      - estimate how the overhead changes with
        # of parameters and their type

 - What happen when I change foo_impl() signature?
   Will it quiely comoile or what kind of error
   will I get. Is the error understandable? Example.

 - Few trivial and non-trivial overcommented complete examples.

 - Info whether there are any additional constructors
   or assignements called for object as parameters.

 - Info about exception safety.

 - Whether it is possible to have both "named params"
   function foo() and the original signature function
   "foo" and whether there could be any problems.
   Can the original signature foo() be externs "C"?

 - How do the usual conversion rules apply? Exceptions,
   examples of these, tricks to make it as safe
   as possible.

 - Do overloads of "named params" functions work?
   Example if they do.

 - How it is with exception specifications - suported,
   unsupported, dangerous here?

 - Does "named params" allocate heap memory? If so,
   in single block?

 - Who are expected end users of the library, examples.

 - I would welocme detailed table what features do
   not work with this or that compiler (more than the
   single line of text now).

 - What is overhead of:
     namespace {
       boost::keyword<name_t> name;
       boost::keyword<value_t> value;
    }
   being in header?

 - Is it possible to reuse tags for different functions?
   Any problems can happen?

 - Does it work with Boost.Any as parameter?

 - Does it work when one used non-default
   calling conventions as __fastcall?

 - Is it possible to export "named params"
   function from DLL? Any problems?

 - Does it work with bind/boost.function/etc?
   Examples.

 - If I use "named params" for the same member
   of both base and derived class: what defaults
   would be used? Is it consistent with default
   handling or 'normal' members?

 - Would it work to provide "named params"
   overload of existing C functions?
   E.g. Win32 API functions?

 - Is it possible to use usual concept checks
   with "named" function parameters? Will it work?

 - The macros is underdocumented and never
   used in tests.

 - How the library works with namespaces: can I put
   the foo_impl into its own inner namespace? Example.

 - What does need to be in header and what can be
   put in implementation file? Example(s).

 - Is there way to make definition of pImpled
   member functions easier?

 - Iteration over parameters is undocumented.
   No examples.

 - There should be info whether the library headers
   can be part of precompiled headers (= has any
   compiler problems?)

- How does the library deals with std::auto_ptr
  parameter? Could there be test for it?

- typedef void (* foo_t)(const char*, float);
  foo_t var = &foo_with_named_params;
  Is this possible/correct/error prone/caught by compiler?

- Is it possible to call function with "named params" from
  another function with "named params" and the same signature
  and pass these params without need to extract them?

    void foo_impl(....) {
      cout << "foo_impl called";
      foo2(???);
    }

- Does the library work with <cstdarg>?

- Can or can not this library be used in a ScopeGuard
  implementation? (I do not have exact idea how it would
  look like.)

_____________________________________________________
9. Number of test should be much higher. These should
   range from very simple ones to complex.
   All possibilities asked in [8] should be covered.

_____________________________________________________
10. named_params.hpp: are the yes_t/no_t needed?
    Can similar types from TypeTraits be used instead?

_____________________________________________________
11. named_params.hpp: the #endif and #else
     should have comment to what they relate.
     The code blocks are quite large and its too easy
     to lost track.

# endif // __EDG_VERSION__ > 300

_____________________________________________________
12. named_params.hpp:
#include <boost/detail/workaround.hpp>
should be there listed explicitly.

_____________________________________________________
13. debugging support: could there be simple function/macro
      which iterates over all named parameters and puts them
      (in readable, pretty form) into single string?
     Such string can then be printed, shown in IDE tooltip
     for examination etc.

_____________________________________________________
EOF