Boost logo

Boost :

From: David Abrahams (dave_at_[hidden])
Date: 2004-11-17 18:41:25

Cromwell Enage <sponage_at_[hidden]> writes:

> Huzzah, my first review!
>> - What is your evaluation of the design?
> Groovy, but not quite the schiznit.

Wow, and the first review I've seen that had real street cred.

> 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>?

Not neccessarily. They should probably be decoupled and included as
needed in each header.

> And would it
> help if each name had an appropriate prefix, like
> "bgl_", to help avoid object collisions?

Not much; I would nest the unnamed namespace inside boost (or a
sub-namespace thereof).

> 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.

I don't understand. Why would you need to be able to get the type in
order to do that?

  p[dependent | some_expression.involving(p[dependency])]

Nevermind, I think I understand now:

  p[dependency | default1]
  p[dependent | some_expression.involving(p[dependency | default1])]

I agree that you ought to that capability.

> May I safely assume that we plan on replacing the
> bgl_named_params mechanism with this one in the
> not-too-distant future?

That's really up to the BGL maintainers, but last I heard yes.

> 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=.

I'm not positively sure what you're saying here. Our design does use
operator| for "providing" defaults. They're never "assigned." To do
that, you would (tautologically) have to use 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".
> actually tied to something in the MPL library, namely
> declaring *that* value,
> 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.

Thanks, I'm glad you tracked that down. We need to figure out what to
do about it.

> I believe the modified <boost/named_params.hpp> header
> attached to this message solves this problem, but I'm
> not sure.

Hey, nifty!

>> - 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.

Yeah, we probably ought to document the operator, usage up front, too.

>> 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.

I'm positive we can do something about it without too much trouble.

Dave Abrahams
Boost Consulting

Boost list run by bdawes at, gregod at, cpdaniel at, john at