NoAvailableAlias / nano-signal-slot

Pure C++17 Signals and Slots
MIT License
407 stars 60 forks source link

Add support for function pointers and lambdas #2

Closed wackou closed 10 years ago

wackou commented 10 years ago

hi there, first of all, thank you for this library, exactly what I was looking for: small, simple, understandable and does what it's supposed to :smile:

One thing is missing, though, which would make it even more perfect:

#include "../nano_signal_slot.hpp"
#include <iostream>
#include <vector>
using namespace std;

void f(int n) {
    cout << "f: " << n << endl;
    return;
}

typedef void (*Fptr)(int);

int main()
{
    Nano::Signal<void(int)> sig;
    sig.connect<f>();

    Fptr g = f;
    sig.connect<g>(); // does not compile

    auto h = [](int n) { cout << "h: " << n << endl; };
    sig.connect<h>(); // does not compile
}

the previous example doesn't compile for me, using Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)

NoAvailableAlias commented 10 years ago

Hello, please refer to the old project documentation here for more information about function objects: https://code.google.com/p/nano-signal-slot/wiki/Documentation#Not_Shown:_Functors_and_Lambdas

Unfortunately your function pointer example won't work because it is not a constant expression. Non-type template parameters work in the compile time realm. However, if you were to use constexpr there, it will work. By that point it kind of defeats the purpose of a function pointer, so I believe there is a good reason this is not possible by design.

Also note that in your last example if you were trying to get the lambda to cast to a function pointer then you would of had to use Fptr instead of auto as the type of h is now some sort of internally defined function object by the time you get to the next line.

To support functors and lambdas add the following to their respective locations:

 ... // add the following to Nano::Function<T_rv(Args...)>

     template <typename L>
     static inline Function bind(L* pointer)
     {
         return { pointer, [](void *this_ptr, Args... args) {
         return (static_cast<L*>(this_ptr)->operator()(args...)); }};
     }
 ... // in Nano::Signal<T_rv(Args...)>

     template <typename L>
     void connect(L* instance)
     {
         Observer::insert(Function::template bind(instance));
     }
 ...
     template <typename L>
     void disconnect(L* instance)
     {
         Observer::remove(Function::template bind(instance));
     }
wackou commented 10 years ago

ah right, I can see now the discrepancy between what's there now, where the function is passed as template parameter, and what I need where the function pointer or lambda is passed as first arg to the connect() method.

The other potential issue with lambdas is that people would connect it, then it goes out of scope, and then the signal is sent to an invalid pointer. I think I understand your reasoning for not allowing functors and lambda, then, however I believe you should mention this in the github readme as I think this would be a quite common question.

In any case, thanks for the quick and detailed answer (and thanks for the library too!)

NoAvailableAlias commented 10 years ago

No problem, glad you find this library useful! I will try to consolidate the documentation in the near future.