Huzzah, my first review!
- What is your evaluation of the design?
Groovy, but not quite the schiznit. A few major questions that will pop up include how users should name the keyword objects for their functions, and where these should be declared. Take the headers-only BGL as an example: should the keyword objects be in an unnamed namespace, according to the named-params docs? Since they may be used so often, should they be defined in a single library-specific header, e.g. <boost/graph/keywords.hpp>? And would it help if each name had an appropriate prefix, like "bgl_", to help avoid object collisions? Also, I second the feature request of getting the type of a named parameter. Numerous BGL algorithms require a graph, an index map that defaults to get(vertex_index_t(), graph), and a color/rank/distance map that defaults to a random_access_iterator_property_map over the index map. Without the type-getting ability (and thus the ability to make temporary assignments), I may have to duplicate the default-assigning code over several places, like I'm doing now retooling my graph algorithms in the sandbox. What I'm really asking for is an elegant way to assign a parameter a default value that is dependent on another parameter. May I safely assume that we plan on replacing the bgl_named_params mechanism with this one in the not-too-distant future? If so, then I feel that these issues need to be addressed.
- What is your evaluation of the implementation?
Mmmm, preprocessing and MPL, like spaghetti and meatballs. So difficult to cook up, but so delicious when served. On the plus side, it beats having to use the undocumented choose_pmap and choose_const_pmap functions to extract property maps from bgl_named_params, among other things. I find operator| to be the most logical operator for assigning defaults, more so than operator& or operator=. The major stumbling block concerns BOOST_NAMED_PARAMS_MAX_ARITY. It looks like it is set arbitrarily to 5, but setting it any higher (before including any header files, as suggested) led to errors like "no such type named boost::mpl::apply9". This meant that BOOST_NAMED_PARAMS_MAX_ARITY is actually tied to something in the MPL library, namely BOOST_MPL_LIMIT_METAFUNCTION_ARITY. IIRC in addtion declaring *that* value, BOOST_MPL_CFG_NO_PREPROCESSED_HEADERS has to be set before including a library header. I doubt an ordinary user would relish following these steps just to pass in more than 5 arguments. And many of the BGL algorithms take in more than 5 arguments. I believe the modified <boost/named_params.hpp> header attached to this message solves this problem, but I'm not sure.
- What is your evaluation of the documentation?
The autogenerating macro should have a more prominent place in the docs. Potential users might get turned off by the number of function overloads they see first, before they get to see the usefulness of this macro. I believe the "best practices" section that someone else came up with (or was that my imagination?) should include suggested ways to resolve the keyword object issues I raised.
- What is your evaluation of the potential usefulness of the library?
It would definitely be useful even outside the BGL. In my multistate mazes project, for example, I have a Parallelepiped class template whose constructor and initializing member function take in something akin to a grid-bag layout (i.e. cell-padding, cell-spacing, and cell-size parameters). This named-parameters mechanism might just supplant the monolithic class template I'm currently using to represent this layout. That my multistate mazes project also uses the BGL is somewhat beside the point...
- Did you try to use the library? With what compiler? Did you have any problems?
I used my main development compiler, GCC 3.2 (MinGW, Dev-C++ 4.9.8.5). The docs state it as a supported compiler, but the test program did not compile until Daniel Wallin's workaround was applied. Oh, well.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
My effort is in-depth, but not as much as I'd like. (I have yet to test any function that might require boost::ref(), for instance).
- Are you knowledgeable about the problem domain?
I'm familiar with the principle of named parameters, but not with many of the technical challenges.
As always, please remember to clearly state whether you believe the library should be accepted into Boost.
The resolution of the design issue concerning parameter-dependent defaults bears more weight on my vote than any of the other pluses or minuses I've mentioned so far. If it can be resolved easily, then I vote yes. Otherwise, not until it is resolved. Cromwell Enage __________________________________ Do you Yahoo!? The all-new My Yahoo! - Get yours free! http://my.yahoo.com