Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Some context: I have code that looks like this (minor issue noted here):

Statement  select("SELECT * FROM People WHERE ID > ? AND ID < ?");
select.execute(1462, 1477, [](int ID, std::string const& person, double item1, float item2){
     std::cout << "Got Row:" 
               << ID     << ", " 
               << person << ", " 
               << item1  << ", " 
               << item2  << "\n";
 });

Anyway this connects to the MySQL DB and starts pulling data from the server. So inside execute I loop over the results and call the lambda for each row:

    template<typename Action, typename ...Args>
    void execute(Args... param, Action action)
    {
        // STUFF TO SET up connection.
        // Start retrieving rows.


        while(row = results->getNextRow())
        {
            call(action, row);
        }
    }

So here row gets a single row from the socket connection with mysql (so it calls the lambda as it receives each row (no pulling the rows into memory first)). So the code I want to review is pulling the data and calling the lambda.

 // Statement::call

    template<typename Action>
    void call(Action action, std::unique_ptr<ResultSetRow>& row)
    {
        typedef CallerTraits<decltype(action)>   trait;
        typedef typename trait::AllArgs         AllArgs;
        Caller<trait::size, 0, AllArgs, Action>::call(action, row);
    }

This utilizes the helper class CallerTraits and Caller to pull the required rows from the stream and then call the lambda:

// CallerTraits
// Get information about the arguments in the lambda

template <typename T>
struct CallerTraits
    : public CallerTraits<decltype(&T::operator())>
{};

template<typename C, typename ...Args>
struct CallerTraits<void (C::*)(Args...) const>
{
    static const int                        size = sizeof...(Args);
    typedef std::tuple<Args...>             AllArgs;
};

Then the Caller:

// Caller::call()
//    Reads the next argument required by the lambda from the stream.
//    An exception will be generated if the next argument on the stream
//    does not match the type expected by the lambda.
template<int size, int index, typename ArgumentTupple, typename Action, typename ...Args>
struct Caller
{
    static void call(Action action, std::unique_ptr<ResultSetRow>& row, Args... args)
    {
        // Get the next argument type required by the lambda.
        // As defined by index. Then remove all ref and const
        // bindings.
        typedef typename std::tuple_element<index, ArgumentTupple>::type    NextArgBase;
        typedef typename std::remove_reference<NextArgBase>::type           NextArgCont;
        typedef typename std::remove_const<NextArgCont>::type               NextArg;

        // Read the next value from the stream.
        NextArg val;
        row->getValue(val);

        // Recursively call Caller::call() (via doCall())
        // To get the next argument we need. All the arguments
        // are accumulated in the var args parameter `args`
        doCall<size-1, index+1, ArgumentTupple>(action, row, args..., val);
    }
};

Specialization when no more args need to be retrieved:

// Specialization of Caller::call() when we have got all the arguments.
// This simply calls the lambda with the arguments we have accumulated.
template<int index, typename ArgumentTupple, typename Action, typename ...Args>
struct Caller<0, index, ArgumentTupple, Action, Args...>
{
    static void call(Action action, std::unique_ptr<ResultSetRow>&, Args... args)
    {
        action(args...);
    }
};

Function to deduce parameter types:

// Function template needed because we
// can not deduce the Args... parameter manually in the call.
// so we let the compiler deduce it for us.
template<int size, int index, typename ArgumentTupple, typename Action, typename ...Args>
void doCall(Action action, std::unique_ptr<ResultSetRow>& row, Args... args)
{
    Caller<size, index, ArgumentTupple, Action, Args...>::call(action, row, args...);
}
share|improve this question
add comment

2 Answers 2

up vote 11 down vote accepted

I find your implementation a bit more complex than necessary. What you want to do is

  1. fetch arguments from your "result set" row by calling its getValue() in a particular order;

  2. use them (as arguments) to call operator() on function object action.

This can be done without recursion in two lines:

Do{row->getValue(std::get<N>(args))...};
action(std::get<N>(args)...);

where args is a tuple.

Range


Ok, now let's step back to see how this is possible. First, we learn how to count from 0 to a given number L, in order to construct range 0, ..., L-1:

// holds any number of size_t parameters
template <size_t... N>
struct sizes { using type = sizes <N...>; };

// given L>=0, generate sequence <0, ..., L-1>
template <size_t L, size_t I = 0, typename S = sizes <> >
struct Range;

template <size_t L, size_t I, size_t... N>
struct Range <L, I, sizes <N...> > : Range <L, I+1, sizes <N..., I> > { };

template <size_t L, size_t... N>
struct Range <L, L, sizes <N...> > : sizes <N...> { };

This is a very common task, actually borrowed from here. There's a better implementation with logarithmic (rather than linear) template depth, but I want to keep it simple here.

"Do"?


Next, an extremely helpful struct lets us evaluate expressions in a given order:

// using a list-initializer constructor, evaluate arguments in order of appearance
struct Do { template <typename... T> Do(T&&...) { } };

But beware, due to a bug since at least version 4.7.0, GCC evaluates in the opposite order, right-to-left. A workaround is to provide a range in the opposite order, L-1, ..., 0, but I'm not doing this here.

Caller


Now, Caller has a generic definition with only two actual parameters, ArgumentTuple and Action. It also reads that tuple's size, say L, and constructs range 0, ..., L-1 in a third parameter:

// generic Caller
template<
    typename ArgumentTuple, typename Action,
    typename Indices = typename Range<std::tuple_size<ArgumentTuple>{}>::type
>
struct Caller;

Finally, a specialization deduces the generated range as variadic size_t parameters N.... A local tuple of type ArgumentTuple is used to store the arguments, and std::get<N> accesses its N-th element. That's it:

// Caller specialization, where indices N... have been deduced
template<typename ArgumentTuple, typename Action, size_t... N>
struct Caller<ArgumentTuple, Action, sizes<N...> >
{
    static void call(Action action, std::unique_ptr<ResultSetRow>& row)
    {
        ArgumentTuple args;
        Do{row->getValue(std::get<N>(args))...};
        action(std::get<N>(args)...);
    }
};

Please note that all the above code compiles but I have not seen it in action since I don't have the database infrastructure. I have just made a minimal definition

struct ResultSetRow { template<typename T> void getValue(T) { } };

So I can only hope it works for you.

I am sorry if this looks like a complete rewrite rather than a review, but I couldn't help it :-) At least I've kept the part of your code where you deduce ArgumentTuple from the lambda.

PS-1 If your ResultSetRow::getValue() is void, then you need to adjust its variadic call to

Do{(row->getValue(std::get<N>(args)), 0)...};

so that each sub-expression evaluates to int rather than void (you cannot have a list-initializer made of void arguments).

PS-2 I suspect you're not really managing resources here, so you don't need std::unique_ptr; a plain ResultSetRow& would suffice.

share|improve this answer
    
That's very interesting. I'll have a look when I get home. –  Loki Astari Mar 25 at 19:30
    
Can you give an example of how you call: Caller<X, Y>::call(action, row); I can quite get all your bits to work together. –  Loki Astari Mar 25 at 20:52
    
@LokiAstari Your Statement::call should call Caller<AllArgs, Action>::call(action, row); –  iavr Mar 25 at 20:58
    
Got it working. Needed to do a few more things to get it compiling. Its not any simpler: pastebin.com/T1Bs3CAG (91 lines) Vs pastebin.com/i67kSr24 (83 lines). I'll do an analysis of the generated code later. –  Loki Astari Mar 25 at 21:32
2  
@LokiAstari Well, I'm a bit self-taught. The best resource I know on templates is the book by Vardevoorde and Josuttis, but even this has only one (limited) chapter on metaprogramming, and unfortunalety there's no edition covering C++11 (yet). Stroustrup's 4th edition of "the C++ programming language" does cover C++11 and again has one chapter on metaprogramming. Anyhow, these are good places to start. Specifically for Do, check variadic templates and look for struct pass. There's some explanation there. –  iavr Mar 26 at 9:16
show 5 more comments

I would have probably applied the following changes:

  • Make size a static constexpr variable in CallerTraits instead of simply static const.
  • Wherever a function simply passes variadic arguments whose types have been deduced, I would have passed args by universal reference and used std::forward to forward the results to the following functions:

    template<int size, int index, typename ArgumentTupple, typename Action, typename ...Args>
    void doCall(Action action, std::unique_ptr<ResultSetRow>& row, Args&&... args)
    {
        Caller<size, index, ArgumentTupple, Action, Args...>::call(action, row, std::forward<Args>(args)...);
    }
    

    It's a bit hard and quite long to explain how it works exactly - you can find a great explanation in the answer linked above -, but the main point is that using this particular recipe implements perfect forwarding:

    template<typename X>
    void foo(X&& arg)
    {
        bar(std::forward<X>(arg));
    }
    

    The type of the parameters of X&&... in foo will have the same const and reference qualifications than the type of the corresponding parameters in bar. Anyway, the link is by far clearer than I am. Simply remember the recipe and that for this recipe to work, the type X has to be deduced by the function; it may not work if X is known from somewhere else.

  • Instead of creating functions that take a std::unique_ptr<ResultSetRow>& parameters, I would have had Caller<...>::call and doCall them take a ResultSetRow& and dereferenced row right away. I don't know what is the exact return type of results->getNextRow() so I won't try to assume anything about it and the type that the main call should take as a parameter.

share|improve this answer
    
Yo use advanced terms I don't understand universal reference can you give examples. –  Loki Astari Mar 25 at 14:34
    
@LokiAstari My bad. I will edit that. –  Morwenn Mar 25 at 14:34
    
MAde all the changes and the unit tests still work. +1 –  Loki Astari Mar 25 at 15:03
1  
@LokiAstari For universal references: isocpp.org/blog/2012/11/… –  Yuushi Mar 26 at 0:04
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.