4

I have a function, Post(), that takes two arguments - a std::string path to listen for requests on, and a std::function<void(Request &, Response &)> to handle an incoming request. Note that I can't modify Post().

For example:

m_server.Post("/wallet/open", [this](auto req, auto res)
{
    std::cout << req.body << std::endl;
}

I'm trying to pass the request through a middleware function, and then forward on to the handler function.

The handler function and the middleware function are member functions. The setup of the Post() binding takes place within a member function of the same class.

This works:

m_server.Post("/wallet/open", [this](auto req, auto res){
    auto f = std::bind(&ApiDispatcher::openWallet, this, _1, _2);
    middleware(req, res, f);
});

However, I'm hooking up a reasonable amount of requests, and it would be nice to abstract this into a function, so we can just call, for example

m_server.Post("/wallet/open", router(openWallet));

I managed to get something like this working, but I can't figure out how to make it work when using member functions. This works great if everything is a free function:

const auto router = [](auto function)
{
    return std::bind(middleware, _1, _2, function);
};

m_server.Post("/wallet/open", router(openWallet))
        .Post("/wallet/keyimport", router(keyImportWallet))
        .Post("/wallet/seedimport", router(seedImportWallet))
        .Post("/wallet/viewkeyimport", router(importViewWallet))
        .Post("/wallet/create", router(createWallet))

However, my attempt to get it working for member functions doesn't work, and I can't really figure out what the error message is telling me:

const auto router = [](auto function)
{
    return std::bind(&ApiDispatcher::middleware, _1, _2, function);
};

m_server.Post("/wallet/open", router(&ApiDispatcher::openWallet))
        .Post("/wallet/keyimport", router(&ApiDispatcher::keyImportWallet))
        .Post("/wallet/seedimport", router(&ApiDispatcher::seedImportWallet))
        .Post("/wallet/viewkeyimport", router(&ApiDispatcher::importViewWallet))
        .Post("/wallet/create", router(&ApiDispatcher::createWallet))

The router function by itself works. I think the issue is I'm not calling the handler functions on this. I tried doing this instead:

m_server.Post("/wallet/open", router(std::bind(&ApiDispatcher::openWallet, this, _1, _2)));

But then I get an error about "Wrong number of arguments for pointer-to-member".

Here's a minimal example to replicate the problem:

#include <iostream>
#include <string>
#include <functional>

using namespace std::placeholders;

class ApiDispatcher
{
    public:
        void middleware(std::string req, std::string res, std::function<void(std::string req, std::string res)> handler)
        {
            // do some processing
            handler(req + " - from the middleware", res);
        }

        void dispatch()
        {
            const auto router = [](auto function)
            {
                return std::bind(&ApiDispatcher::middleware, _1, _2, function);
            };

            /* Uncommenting this line breaks compilation */
            // Post("/wallet/open", router(&ApiDispatcher::openWallet));
        }

        void openWallet(std::string req, std::string res)
        {
            std::cout << req << std::endl;
        }

        void Post(std::string method, std::function<void(std::string req, std::string res)> f)
        {
            // wait for a http request
            f("request", "response");
        }    
};

int main()
{
    ApiDispatcher server;
    server.dispatch();
}

Thanks. Sorry the post was so long.

max66
  • 65,235
  • 10
  • 71
  • 111
Zpalmtree
  • 1,339
  • 8
  • 14
  • You seem to be forgetting that a member function needs an object. Once a member function is wrapped with `bind`, you pass that object as a first argument. Alternatively, make `openWallet` and `middleware` static member functions – Piotr Skotnicki Nov 21 '18 at 22:15

4 Answers4

2

Your router function needs to return a function that will call the specified member function with a specific instance of the class.

void dispatch()
{
    const auto router = [this](auto function) {
        return std::bind(function, this, _1, _2);
    };

    Post("/wallet/open", router(&ApiDispatcher::openWallet));
}

Or, and I missed this originally, to route everything through the middleware function, easiest I could find was.

void dispatch()
{
    const auto router = [this](auto function) {
        return [this, function](auto a, auto b) {
            middleware(a, b, std::bind(function, this, _1, _2));
        };
    };

    Post("/wallet/open", router(&ApiDispatcher::openWallet));
}

Or, if you'd rather use bind, then you'll have to force the inner bind to be a function first, and do.

void dispatch()
{
    const auto router = [this](auto function) {
        std::function<void(std::string, std::string)> func = std::bind(function, this, _1, _2);
        return std::bind(&ApiDispatcher::middleware, this, _1, _2, func);
    };

    Post("/wallet/open", router(&ApiDispatcher::openWallet));
}
evan
  • 1,463
  • 1
  • 10
  • 13
  • Perhaps I'm missing something in this answer, but the middleware function is not getting called here - the output is `request`, not `request - from the middleware`. – Zpalmtree Nov 21 '18 at 22:19
  • You're right. I missed the part about going through middleware... Updated my answer. – evan Nov 21 '18 at 22:32
  • I quite like this one as we don't have to specify the std::bind in the router call, keeping it a bit cleaner. Thanks! – Zpalmtree Nov 21 '18 at 22:43
2

I find it easier to reason with lambdas, and from what I understand they are to be prefered over std::bind for performance reasons.

void dispatch()
{
    Post("/wallet/open", [&](std::string req_1, std::string res_1){
        middleware(req_1, res_1, [&](std::string req_2, std::string res_2){
            openWallet(req_2, res_2);
        });
    });
}

Each call to a member function is wrappen in a lambda here. You should probably be using const std::string& as arguments to avoid copies for all the calls.

Another thing to point out for performance. As long as you are only forwarding function calls (you are not storing the callable for later use) you can change middleware and Post to templates. The callable would be the template parameter and you will not have to pay the overhead of std::function.

template <typename F>
void middleware(std::string req, std::string res, F handler)
{
    // do some processing
    handler(req + " - from the middleware", res);
}

template <typename F>
void Post(std::string method, F f)
{
    // wait for a http request
    f("request", "response");
}

Edit

To extract some of the logic in the way I think you mean, you pass a pointer-to-member-function.

void dispatch()
{
    auto const router = [&](auto m_fptr) {
        return [&, m_fptr](std::string req, std::string res) {
            middleware(req, res, [&](std::string req_2, std::string res_2) {
                (this->*m_fptr)(req_2, res_2);
            });
        };
    };

    Post("/wallet/open", router(&ApiDispatcher::openWallet));
}
super
  • 12,335
  • 2
  • 19
  • 29
  • This works great, thank you! Is it possible to extract some of the logic into a separate function? I'm not sure if I prefer this method or the bind method I was using - both aren't great readability wise, unfortunately. – Zpalmtree Nov 21 '18 at 22:25
  • Most likely, I'm not entirely sure what you mean though. Extract which part? – super Nov 21 '18 at 22:27
  • I want to make quite a few Post() calls, so I'm wanting the second argument to be as lightweight as possible. something like `Post("/wallet/open", router(openWallet))` would be best, where the router function handles the nested lambdas we're using here. I can't figure out how to pass a member function (openWallet) to router without doing the lambda inline. – Zpalmtree Nov 21 '18 at 22:30
  • At the very least you would have to change the `router` call to also take the pointer to the object on which to call `openWallet`. Something like `router("/wallet/open", router(this, &ApiDispatcher::openWallet);`. – super Nov 21 '18 at 22:36
  • Not sure how that would work - We could pass the this argument, along with the function, but how would we call the function on the specific class instance? Perhaps with a function pointer, somehow? – Zpalmtree Nov 21 '18 at 22:42
  • @Zpalmtree An edit to show an example of how it could be done. It feels a bit strange to me though instinctively since `this` is pre-captured in `dispatch` and then used on the pointer passed in later. But for the code shown it is safe. – super Nov 21 '18 at 23:00
1

Not sure... but seems to me that your idea was writing this or something similar

    const auto router = [this](auto function)
    {
        std::function<void(std::string req, std::string res)> fn
           = std::bind(function, this, _1, _2);
        return std::bind(&ApiDispatcher::middleware, this, _1, _2, fn);
    };

But I suggest to follow the lambda-inside-lambda solution proposed by super.

max66
  • 65,235
  • 10
  • 71
  • 111
1

The root of the problem is because of the parameter on the router lambda and how the std::bind inside the the lambda will interpret it.

let assume that you've fixed the call to router inside dispatch, replacing:

Post("/wallet/open", router(&ApiDispatcher::openWallet));

by the appropriate form:

Post("/wallet/open", router(std::bind(&ApiDispatcher::openWallet, this, _1, _2)));

then by cppreference, the function parameter of the lambda router will be deduced to be a bind expression and as consequence the std::bind inside router will see that this parameter satisfy std::is_bind_expression<T>::value == true and no conversion to std::function will be attempted when invoking middleware.

To fix it you need to specify explicitly the type of function parameter on router lambda, as:

const auto router = [this](std::function<void(std::string req, std::string res)> function)
{
    return std::bind(&ApiDispatcher::middleware, this, _1, _2, function);
};

Doing it this way the implicit conversion of

std::bind(&ApiDispatcher::openWallet, this, _1, _2)

to std::function takes place before calling

std::bind(&ApiDispatcher::middleware, this, _1, _2, function)

or alternatively you keep the auto, and trigger the implicit conversion inside the lambda:

const auto router = [this](auto function)
{
    std::function<void(std::string req, std::string res)> func = function;
    return std::bind(&ApiDispatcher::middleware, this, _1, _2, func);
};
Jans
  • 11,064
  • 3
  • 37
  • 45
  • Perfect! I knew there was a missing `this` somewhere, but wasn't sure where to put it. This is exactly what I was looking for. – Zpalmtree Nov 21 '18 at 22:33
  • I've gotten pretty used to using `auto` in lambda's with C++17, as I find it makes my code much more readable - I wasn't aware it could break conversions like this. – Zpalmtree Nov 21 '18 at 22:35
  • Yeah, You can keep the `auto` but still need to trigger the implicit conversion before the second `std::bind` ... (check the edit). – Jans Nov 21 '18 at 22:43