From: David Abrahams (dave_at_[hidden])
Date: 2004-11-23 14:26:00
Doug Gregor <dgregor_at_[hidden]> writes:
> My review of the named parameters library follows:
>
>> - What is your evaluation of the design?
>
> The design is good. The library improves the syntax of named
> parameters greatly relative to existing solutions, makes it trivial to
> define new keywords, provides a simple DSL for specifying default
> parameters, and has some useful shortcuts.
>
> I have a few issues with the interface:
>
> 1) We need a params.has(k) member, that returns mpl::true_ when the
> parameter is there and mpl::false_ when the parameter is not
> there. That lets us dispatch either statically or dynamically based
> on the existence of an argument for keyword k.
Seems like that interface makes static dispatch unneccessarily hard.
How about a metafunction:
has_param<Params, k>
such that
has_param<Params, k>
is derived from either mpl::true_ or mpl::false_ and
has_param<Params, k>::type
is one of mpl::true_ or mpl::false_.
??
> 2) The macro BOOST_NAMED_PARAMS_FUN is okay when all parameters may be
> named parameters. In my use cases, there are always several
> positional-only parameters for which I do *not* want the option to
> rename them, and may want to be more picky about how they are
> passed to the function. For instance, I want to be able to take a
> "Graph& g" as the first parameter. One way to address this would be
> another variant of this macro, which I'll lamely call
> BOOST_NAMED_PARAMS_FUN_EXT, that might work like this:
>
> BOOST_NAMED_PARAMS_FUN_EXT(void, page_rank, 0, 3, detail::page_rank_keywords,
> (2, (typename Graph, typename RankMap)),
> (2, ((Graph&, g), (RankMap rank))))
>
> The first new argument is a preprocessor array giving the extra
> template parameters that should be introduced into the function. The
> second new argument is also a preprocessor array, this one containing
> the parameter types and names (each as tuples). I believe there is
> enough information here to generate, e.g.,
>
> template<typename Graph, typename RankMap>
> void page_rank(Graph& g, RankMap rank)
> { return page_rank_with_named_params(g, rank,
> detail::page_rank_keywords()()); }
>
> template<typename Graph, typename RankMap, typename T0>
> void page_rank(Graph& g, RankMap rank, const T0& a0)
> { return page_rank_with_named_params(g, rank,
> detail::page_rank_keywords()(a0)); }
>
> etc...
Probably something like that could work. But my intuition tells me
this is a good candidate for vertical repetition:
#ifndef BOOST_PP_IS_ITERATING
...
# define BOOST_NAMED_PARAMS_SETUP(whatever.hpp, 2, 5)
# include BOOST_NAMED_PARAMS_GENERATE
#else
template <class Graph, class RankMap BOOST_NAMED_PARAMS_TYPES>
void page_rank(Graph& g, RankMap rank BOOST_NAMED_PARAMS_ARGS)
{
return page_rank_with_named_params(
g, rank, BOOST_NAMED_PARAMS_KEYWORDS);
}
#endif
Otherwise it's just too ugly and unreadable, not to mention what
happens when types contain commas.
> 3) Partly due to the missing "params.has(p)", we're lacking the
> ability to determine the type of an expression like
> "params[index_map | get(vertex_index, g)]" without passing it to
> another function. I'd gladly give up some clarity in the handling
> of default parameters if I could get at this information. I'd even
> go for an auto-like macro:
>
> BOOST_NAMED_PARAMS_AUTO(index, index_map, get(vertex_index, g));
Yikes; that one is hard without some kind of typeof support.
How about:
BOOST_NAMED_PARAMS_AUTO(index, index_map, some-type-expression);
??
You already have some way to look up the type of
get(vertex_index, g)
right?
> If we had both (2) and (3), it would be feasible to implement
> algorithms such as those in the graph library without manually
> introducing several layers of forwarding functions used only to
> determine types.
>
> 4) Using the name "restrict" in a public interface feels like a really
> bad idea given its status in C99...
Yeah, we should change that.
> The documentation was a little bit lacking. There were a few specific
> things I would like to see:
>
> 1) A "guide for library writers" that helps one organize a library
> properly for named parameters. For instance, where should we put
> keywords? I know we should use an anonymous namespace, but should
> that be in namespace boost? In a "keywords" subnamespace?
Okay.
> 2) Keywords are stated to be of type "boost::keyword<keyword
> type>". Can we derive from the appropriate boost::keyword<...>
> instead? This is important to the BGL for backward compatibility
> reasons.
I honestly don't know. If we can, we should enable it.
> 3) Describing that operator, exists would be helpful.
Definitely.
-- Dave Abrahams Boost Consulting http://www.boost-consulting.com