views:

500

answers:

7

I have a custom Menu class written in C++. To seperate the code into easy-to-read functions I am using Callbacks.

Since I don't want to use Singletons for the Host of the Menu I provide another parameter (target) which will be given to the callback as the first parameter (some kind of workaround for the missing "this" reference).

Registration-Signature

AddItem(string s, void(*callback)(void*,MenuItem*), void* target = NULL)

Example of a Registration

menu->AddItem(TRANSLATE, "translate", &MyApp::OnModeSelected);

Example of a Handler

/* static */
void MyApp::OnModeSelected(void* that, MenuItem* item) {
    MyApp *self = (MyApp*)that;
    self->activeMode = item->text;
}

Is there anything one could consider dirty with this approach? Are there maybe better ones?

+8  A: 

I like your approach. One alternative would be to declare an interface, which is in some sense the "OO equivalent" of a callback:

class IMenuEntry {
public:
    virtual void OnMenuEntrySelected(MenuItem* item) = 0;
};

The registration signature would become

AddItem(string s, IMenuEntry * entry);

And the method implementation

class MyApp : public IMenuEntry {
public:
    virtual void OnMenuEntrySelected(MenuItem* item){
        activeMode = item->text;
    }
}

The interface approach would allow you to avoid the "void * workaround" for the missing this pointer.

Paolo Tedesco
+1  A: 

I don't see anything wrong except that the function pointer signature is hard to read. But, I would probably observer pattern to achieve this.

Naveen
+3  A: 

You could take a look at using boost::bind.

menu->AddItem(TRANSLATE, 
              "translate", 
              boost::bind( &MyApp::OnModeSelected, this, _1, _2 ));
Alan
+1  A: 

I'd highly recommend looking at boost::function and boost:bind for this. Learning it will make your function binding a hundred times easier.

Nat Ryall
also, don't forget boost::lambda. It can sometimes let you away without a callback altogether!
EFraim
boost::signals is better for this task.
lionbest
+10  A: 

Your approach requires the callback functions to either be free functions or static members of a class. It does not allow clients to use member functions as callbacks. One solution to this is to use boost::function as the type of the callback:

typedef boost::function<void (MenuItem*)> callback_type;
AddItem(const std::string& s, const callback_type& callback = callback_type());

Clients can then use boost::bind or boost::lambda to pass in the callback:

menu->AddItem("Open", boost::bind(&MyClass::Open, this));

Another option is to use boost::signals which allows multiple callbacks to register for the same event.

Bojan Resnik
MSalters
You should not use static members of the class as a general callback (they do not have a defined ABI). The boost::function is just a generalization of declaring an interface (the boost::function just uses the interface operator() ). I would prefer my interfaces to be a bit more explicit in this case so that you do not pass an inappropriate method back (ie get the compiler to validate the callback) as described by orsogufo
Martin York
+1  A: 

Read this white-paper. It builds various techniques for a callback mechanism by analysing the performance, usability and other tradeoffs in quite a detail. I found it a hard read though :-(

Abhay
A: 

Your could use a functor to encapsulate your callback. This would allow you to use either a C-style function or an object interface to provide the callback.

David Coufal