C++ binding overloaded member function and passing as parameter

Question

I have a class with two functions which start and stop a collection of things. As the two functions are identical except that they ultimately call a start or stop function on each thing respectively, I would like to refactor the code so that I move the body of the code to general function and my start and stop collections call this passing in an extra parameter which is a the function they must call to start or stop.

Naturally, there are lots of std::bind() tutorials and examples online, but I have not found any article or question/answer here that covers all the following specific constraints that I face:

The function being bound is a member which complicates things; it is also non-const
The function being bound is also overloaded so must be distinguished properly
Most of the std::bind() examples use auto but in this case I need to know the type of the return from std::bind() in order to pass as a parameter to action_widgets()
There are two bools, a & b, which are effectively constant and could be bound into the function though I've not yet done that here. One thing at a time.

Here's an example of what I'm trying to achieve:

#include <string>
#include <vector>
#include <functional>

struct Processor {

    using WidgetActionFunction = bool(Processor::*)(const std::string&,
                                                    bool, bool);

    // Function wrapper 
    using WidgetActionWrapper = std::function<bool(Processor&,
                                                   const std::string&, bool, bool)>;

    // These functions in reality are tied heavily to the class and are quite
    // large. They cannot easily be made static or free.
    bool stop_widget(const std::string& key, bool a, bool b) { return true; }
    bool start_widget(const std::string& key, bool a, bool b) { return true; }

    // Just to make life difficult, there are some overloads, which we're not
    // interested in.
    bool stop_widget(int event, bool a, bool b) { return true; }
    bool start_widget(int event, bool a, bool b) { return true; }

    // I created this function because start_widgets() and stop_widgets() were
    // identical except that internally they call start_widget() and stop_widget()
    // respectively. I want the main body of the code to be here and for the
    // appropriate function to be passed in.
    void action_widgets(std::vector<std::string>& widgets,
            bool a, bool b, WidgetActionWrapper& func) {
        std::vector<std::string> valid_widgets;
        valid_widgets.reserve(widgets.size());
        for (const auto& widget : widgets) {
            if (func(*this, widget, a, b)) {  // This is where func() gets invoked.
                valid_widgets.push_back(widget);
            }
        }
        std::swap(widgets, valid_widgets);
    }

    void start_widgets(std::vector<std::string>& widgets, bool a, bool b) {
        WidgetActionWrapper func =
            std::bind(static_cast<WidgetActionFunction>(&Processor::start_widget),
                      this, std::placeholders::_1, a, b); // compilation fails here.
        action_widgets(widgets, a, b, func);
    }

    void stop_widgets(std::vector<std::string>& widgets, bool a, bool b) {
        // Very similar to start_widgets() but calls with bound stop_widget()
        // instead.
    }
};

int main()
{
    return 0;
}

When compiled I get the following error:

error: conversion from ‘std::_Bind_helper<false, bool (Processor::*)(const std::basic_string<char>&, bool, bool), Processor* const, const std::_Placeholder<1>&, bool&, bool&>::type {aka std::_Bind<std::_Mem_fn<bool (Processor::*)(const std::basic_string<char>&, bool, bool)>(Processor*, std::_Placeholder<1>, bool, bool)>}’ to non-scalar type ‘Processor::WidgetActionFunctor {aka std::function<bool(Processor&, const std::basic_string<char>&, bool, bool)>}’ requested

So clearly, my function wrapper alias down't match what std::bind() is returning but where did I go wrong?

One last caveat or two: Because this is for a corporate client, I am restricted to C++11 solutions (although solutions for the benefit of others are appreciated) and also, though I'm keen on a simpler solution using lambdas, I'm lead to believe by colleagues that this may be equally tricky and in any case, from a technical perspective, I'm keen to know what I've got wrong.

"I need to know the type of the return from `std::bind()` in order to pass as a parameter to `action_widgets()`" but you're not passing any type information to `action_widgets`. And please create a [mcve]. — Passer By, May 01 '19 at 10:00
Sorry I don't understand your point. This is a MCVE, though I will add `main()` if that helps. The fourth parameter of `action_widgets()` is the bound function. This doesn't compile because the type I'm currently using, `WidgetActionWrapper` does not match what `std::bind()` is returning. I need to understand what it's returning so I can alter the type of the 4th parameter accordingly. — Component 10, May 01 '19 at 10:07
Use lambdas instead of `std::bind`. With that said, you're only passing one placeholder to bind, so the function object you get out will have the signature `bool(int)` and then you try to call (and pass) it with all four parameters. — super, May 01 '19 at 10:29

Michael Kenzel · Answer 1 · 2019-05-01T10:44:33.987

I don't think there is a need here for lambdas or std::bind, and especially not std::function and all the overhead it would introduce. You could simply use a member function template that is given a pointer to the actual member function to call on each widget as a template argument, for example:

struct Processor
{
    bool stop_widget(const std::string& key, bool a, bool b) { return true; }
    bool start_widget(const std::string& key, bool a, bool b) { return true; }

    bool stop_widget(int event, bool a, bool b) { return true; }
    bool start_widget(int event, bool a, bool b) { return true; }

    template <bool (Processor::* func)(const std::string&, bool, bool)>
    void action_widgets(std::vector<std::string>& widgets, bool a, bool b)
    {
        std::vector<std::string> valid_widgets;
        valid_widgets.reserve(widgets.size());
        for (const auto& widget : widgets)
        {
            if ((this->*func)(widget, a, b))
            {
                valid_widgets.push_back(widget);
            }
        }
        std::swap(widgets, valid_widgets);
    }
};

and then

processor.action_widgets<&Processor::start_widget>(widgets, true, false);
processor.action_widgets<&Processor::stop_widget>(widgets, true, false);

live example here

This will basically just cause the compiler to generate your original start_widgets and stop_widgets functions for you, just as if you had written them by hand, no additional runtime overhead. Since the template argument asks for the function of the proper type, the compiler should correctly figure out which of the overloaded functions to use…

Using the template argument to pass a lambda would make the code less verbose. Do you think that would be worse for performance? I don't. — super, May 01 '19 at 10:49
@super performance would almost certainly be exactly the same. I'm not sure how exactly it would be less verbose, however… — Michael Kenzel, May 01 '19 at 10:50
You could remove `a` and `b` completely, and `bool (Processor::* func)(const std::string&, bool, bool)` => `typename F`. — super, May 01 '19 at 10:51
@super well, you could do that indeed. This would then allow basically anything to be used in `action_widgets`, not just suitable methods of `Processor`. I'm not sure whether that would still align with the intended purpose or not. If that's fine, it's probably an even better approach. I find the whole construction of this `Processor` class rather strange to be honest. Once we get to a point where `action_widgets` should work with any arbitrary callable, I think we do have to start asking the question of why this is a method of a class rather than just a generic algorithm to begin with… — Michael Kenzel, May 01 '19 at 10:55

score 3 · Accepted Answer · answered May 01 '19 at 10:36

You can think of std::bind as taking off the first few arguments when you assign to a std::function.

For example, this:

bool(Processor::*)(const std::string&, bool, bool);

// Which is this:
class Processor { bool f(const std::string&, bool, bool); }
decltype(&Processor::f)

is assigned to a std::function<bool(Processor&, const std::string&, bool, bool)>.

When you bind it to a Processor& (in your case, *this, like std::bind(&Processor::f, *this)), it should now be assigned to a std::function<bool(const std::string&, bool, bool)> (Because the bind gets rid of the Processor& argument).

There are two fixes here. Don't bind:

WidgetActionWrapper func =
    std::bind(static_cast<WidgetActionFunction>(&Processor::start_widget),
              *this, std::placeholders::_1, a, b); // compilation fails here.
// becomes
WidgetActionWrapper func = &Processor::start_widget;

Or change WidgetActionWrapper to be correct after binding:

// *this and the two bool parameters have been bound, so you only need a string to call
using WidgetActionWrapper = std::function<bool(const std::string&)>;
// (And then `func(*this, widget, a, b)` to `func(widget)`)

C++ binding overloaded member function and passing as parameter

2 Answers2