2
\$\begingroup\$

After looking at a post on CR I found that pointers to member functions are incredibly hard to use, especially as callbacks. The current solution doesn't solve the problem mentioned in the linked post, but tries to get rid of the type erasure (i.e. std::function<>) by making the target type explicit. I tried to solve lifetime/ownership issue with something else than std::shared_ptr<>, but found that it is impossible without relying on programmer to take care of that. So, I ended up with std::shared_ptr<> as a solution.

So, here is the code:

#pragma once
#include <memory>
#include <utility>

template <typename T, typename Func>
class member_callback;

template <typename T, typename R, typename ... ArgTypes>
class member_callback<T, R(ArgTypes...)>
{
    std::shared_ptr<T> object;
    R(T::* callback)(ArgTypes ...);
public:
    using function_pointer = R(T::*)(ArgTypes ...);

    member_callback(const std::shared_ptr<T>& ptr, R(T::* function)(ArgTypes...)) :
        object(ptr),
        callback(function)
    {}

    member_callback(std::shared_ptr<T>&& ptr, R(T::* function)(ArgTypes...)) :
        object(std::move(ptr)),
        callback(function)
    {}

    R operator()(ArgTypes&& ... args)
    {
        return ((*object).*callback)(std::forward<ArgTypes>(args)...);
    }

    std::shared_ptr<T> get()
    {
        return object;
    }

    function_pointer get_function()
    {
        return callback;
    }
};

It does one tradeoff though, you can't make a vector of them and call member functions of completely unrelated classes. Nevertheless, you'll still be able to invoke virtual functions, and they will work as expected according to this answer.

From the first glance it might look useless. But it is not! It becomes extremely useful when you're making a menu (1. ... 2. ... 3. ...) and want to invoke a function on the single object. So, here is the example:

#include "member_callback.hpp"
#include <iostream>
#include <vector>

class integer
{
    int x;
public:
    integer():
            x(0)
    {}

    integer& operator++()
    {
        ++x;
        return *this;
    }

    integer& operator--()
    {
        --x;
        return *this;
    }

    friend std::ostream& operator<<(std::ostream& os, const integer& i);
};

std::ostream& operator<<(std::ostream& os, const integer& i)
{
    os << "Currently number is: " << i.x << '\n';
}

void print_menu()
{
    static auto message = "Type the specified number to invoke the command.\n";
    static auto menu = "0. Increment\n1. Decrement\nAnything else to exit\n";

    std::cout << message << menu;
}

int main()
{
    auto object = std::make_shared<integer>();

    std::vector<member_callback<integer, integer&()>> actions{{object, &integer::operator++},
                                                              {object, &integer::operator--}};

    std::cout << "Welcome to dummy program!\n";
    std::cout << *object;

    unsigned int response =  -1;
    print_menu();
    std::cin >> response;
    while (response < 2)
    {
        actions[response]();
        std::cout << *object;
        print_menu();
        std::cin >> response;
    }

    std::cout << "Goodbye!\n";
}

If code will ever get changed you can find it here.

I would like a review to focus on the easy of use. Also, does the code provide fine grained control? It will require dealing with std::shared_ptr<>, but I think it is mostly ok. Any other review points are welcome!

\$\endgroup\$
  • \$\begingroup\$ Ahh. I see it is a shared_ptr you will need to dereference it. \$\endgroup\$ – Martin York Dec 5 '16 at 18:08
3
\$\begingroup\$

Overall, the design looks reasonable. However, I can't shake the feeling that Lambda expressions are a good (maybe even better) fit here.

For example, instead of

member_callback(object, &integer::--);

you could use a Lambda expression:

[&] () { return object--; }

This is almost as short, and has some additional advantages:

  • You aren't effectively "reinventing the wheel". Although I wouldn't say your member_callback class is precisely the same as a lambda, it's pretty close.
  • I'm not qualified to say whether your code conforms to the C++ 11 specification. But assuming it does, you are relying on relatively obscure portions of the specification. This has some practical problems:

    • Others might have a hard time understanding and verifying your code.
    • Compilers often don't conform as strictly to the spec as we would like. So you might find that your code is not portable, even if it does conform the the spec.
  • You noted that one of the drawbacks of your approach is that you cannot mix functions to different types of objects. With lambdas, you can combine functions that work on different types of objects. You aren't just limited to objects of a single (base) type (in your case, integer). Likewise, you can define functions that operate on more than one object, or even none at all (e.g., produce a constant value). Or, you could combine functions.
  • Lambdas open the door to higher-order approaches. For example, if you Curry the member_callback function, you can partially instantiate a callback:

    • With the order you have (object, function), you can construct a factory for working on a single object. For example (eliding types for brevity):

      factory = member_callback_curried(object);
      
      // Sometime later, perhaps in a different scope
      dec = factory([] (integer& obj) { return obj--; });
      inc = factory([] (integer& obj) { return obj++; });
      
    • Reversing the order to (function, object), you can construct a factory for working with the specified function:

      factory = callback_member_curried([&] (integer& obj) { return obj-- });
      
      // Sometime later, perhaps in a different scope
      dec_o1 = factory(obj1); // function that decrements obj1
      dec_o2 = factory(obj2); // function that decrements obj2
      
\$\endgroup\$
  • \$\begingroup\$ @Incomputable, why do you think lambdas are incompatible with std::shared_ptr<>? \$\endgroup\$ – Nathan Davis Dec 5 '16 at 19:36
  • \$\begingroup\$ @Incomputable, yes, it probably makes the most sense to take them by value. I don't know why you shouldn't be able to store shared_ptrs in a vector of type std::vector<shared_ptr<std::function<...>>. \$\endgroup\$ – Nathan Davis Dec 5 '16 at 20:30
  • 2
    \$\begingroup\$ @Incomputable, first of all, this is the first time you have mentioned std::function<>. Second, if that is the "whole point", then stackoverflow would be a better fit for that question. You asked for a review of your code, and that is what I gave. \$\endgroup\$ – Nathan Davis Dec 5 '16 at 21:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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