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.

2 Answers

3
Artyer On Best Solutions

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)`)
4
Michael Kenzel On

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…