HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

C++ EventHandler class, invoking & arguments

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
invokingeventhandlerclassarguments

Problem

This is my EventHandler class, which is extremely simple and can be used on lambdas and similar:

template
class Event
{
private:
    typedef std::function Function;
    Function _func;

public:

    void CallFunc(Ax... ax)
    {
        if (_func)
        {
            (_func)(ax...);
        }
    }

    void operator+=(Function func)
    {
        _func = func;
    }

};


I'm looking to find improvements as it's basically as simple as it gets.

Example:

Event myEvent;
myEvent += [](int arg) { printf("%i", arg); };

myEvent.CallFunc(15);


Would print out 15.

Solution

I don't like that you use operator += to set the function. += suggests that I can append a bunch of functions and they all get called when the event happens, which is not the case. myEvent.setFunction([](int){}); would be better.

It would make sense to take advantage of move semantics. The lambda that gets passed into myEvent is moved into func and then copied into _func, when 2 moves would have been enough:

void operator+=(Function func)
{
    _func = std::move(func);
}


Better yet: Make it a template and perfect forward the argument to have only 1 move:

template
void operator+=(Fun &&func)
{
    _func = std::forward(func);
}


You get somewhat worse error messages but most likely better performance.

Same goes for CallFunc, it should take a variadic template argument that gets perfect forwarded into the call for efficiency.

Speaking of efficiency, the flexibility of std::function comes at a price. It is known at compile time what types func can be, so _func should be exactly the same type without type erasure and virtual function calls. You can make that work with a template Event make_event(Fun &&fun) style function to get the type of the function and create an Event with the appropriate template parameters, but you lose the ability to specify the function later.

There are extra parenthesis in the call (_func)(ax...);. Maybe it is a style thing, but I would write _func(ax...); instead.

Oh, and printf is not very C++y, but it was just an example.

Code Snippets

void operator+=(Function func)
{
    _func = std::move(func);
}
template<class Fun>
void operator+=(Fun &&func)
{
    _func = std::forward<Fun>(func);
}

Context

StackExchange Code Review Q#113797, answer score: 3

Revisions (0)

No revisions yet.