views:

142

answers:

3

I am trying to create a generic "callback" object that will hold arbitrary data and invoke member functions of related classes. Due to internal policy, I cannot use Boost.

The callback object looks like this:

template<typename Object, typename Data>
class Callback
{
public:
  typedef void (Object::*PHandler)(Callback*);
  Callback(Object* obj, PHandler handler) : pObj(obj), pHandler(handler) {}
  Callback& set(PHandler handler) { pHandler = handler; return *this; }
  void run() { (pObj->*pHandler)(this); }

public:
  Data data;

protected:
  Object* pObj;
  PHandler pHandler;
};

And the class it works on:

struct Object1
{
  struct Data { int i; };

  typedef Callback<Object1, Data> Callback1;

  void callback(Callback1* pDisp) { printf("%cb\n", pDisp->data.i); }

  void test()
  {
    Callback1 cb(this, &Object1::callback);
    cb.data.i = 1;
    cb.run();
  }
};

The following test works as expected:

Object1 obj1;
obj1.test();

So far so good.

However, when a coworker tried to derive from the Callback class instead of using a typedef, they got compilation errors due to incompatible pointers:

struct Object2
{
  struct Data { int i; Data(int j) { i = j; } };

  class Callback2 : public Callback<Object2, Data>
  {
    Callback2(Object2* obj, PHandler handler, int i) : Callback(obj, handler) { data.i = i; }
  };

  void callback(Callback2* pDisp) { printf("%cb\n", pDisp->data.i); }

  void test()
  {
    Callback2 cb(this, &Object2::callback, 2);
    cb.run();
  }
};

I tried using the "curiously recurring template pattern" in the Callback class and managed to get derived classes working, but it broke code that used the typedef method.

My question is:

How can I modify the Callback class to work with both cases, and without requiring extra work on the part of the user of the class?

Thank you.

A: 

Due to internal policy, I cannot use Boost.

Quit ;)

However, when a coworker tried to derive from the Callback class instead of using a typedef

Shoot them.

I feel for you, I really do.

Noah Roberts
Come on Noah, you can do better than that, given the 222 answers to your name (as of now). Make this one the 223rd.
Alex O
OK, if you don't want to quit or murder your coworkers then simply make sure to document all the time you wasted messing with this stuff when the tools that do what you want were readily available. IMHO anyone that says, "I insist on subclassing rather than using typedef," should be shot. Any court would find it justifiable.
Noah Roberts
A: 

You have to pass the class type of the derived. To not break the typedef-way, you can can give that parameter a default value. Something like the following should work

template<typename Object, typename Data, typename Derived = void>
class Callback;

namespace detail {
template<typename Object, typename Data, typename Derived>
struct derived { 
  typedef Derived derived_type;
};

template<typename Object, typename Data>
struct derived<Object, Data, void> { 
  typedef Callback<Object, Data, void> derived_type;
};
}

template<typename Object, typename Data, typename Derived>
class Callback : detail::derived<Object, Data, Derived>
{
  typedef typename 
    detail::derived<Object, Data, Derived>::derived_type
    derived_type;

  derived_type &getDerived() {
    return static_cast<derived_type&>(*this);
  }

public:
  // ... stays unchanged ...

  derived_type& set(PHandler handler) { 
    pHandler = handler; return getDerived(); 
  }
  void run() { (pObj->*pHandler)(&getDerived()); }

  // ... stays unchanged ...
};

Alternatively you can simply have two classes for this. One for inheritance and one if you don't inherit. The first is for inheritance

template<typename Object, typename Data, typename Derived>
class CallbackBase
{
  typedef Derived derived_type;

  derived_type &getDerived() {
    return static_cast<derived_type&>(*this);
  }

public:
  // ... stays unchanged ...

  derived_type& set(PHandler handler) { 
    pHandler = handler; return getDerived(); 
  }
  void run() { (pObj->*pHandler)(&getDerived()); }

  // ... stays unchanged ...
};

And the second is for non-inheritance. You can make use of the base-class for this

template<typename Object, typename Data>
struct Callback : CallbackBase<Object, Data, Callback<Object, Data> > {
  Callback(Object* obj, PHandler handler) : Callback::CallbackBase(obj, handler) {}
};
Johannes Schaub - litb
Thanks, it looks like a good solution. I'll study it some more...
Alex O
The stuff you put in "namespace detail" was what I was missing. Thanks.
Alex O
+1  A: 

It is unfortunate that you work for a company which advocates reinventing wheels and using bronze age C++. However, given the circumstances and the code you posted, there is a fairly simple solution without making too many changes. I've had to do similar things before not because of company policy but because we were developing a software development kit and we did not want to require that users of the SDK have a specific version of boost installed.

BTW, I believe what you are really after is a signals and slots mechanism: I suggest studying it if you want to make this design a bit more conforming to well-established solutions.

I took a few liberal assumptions: the primary one being that Data does not have to be stored in the callback and can instead be passed by the sender. This should be a more flexible design. It is still very simple - you can take this a lot further (ex: variable-number of arguments and custom return types, a signal class, deferred notifications, etc).

template<typename Data>
class CallbackInterface
{
public: 
    virtual ~CallbackInterface() {}
    virtual void run(const Data& data) = 0;
    virtual CallbackInterface* clone() const = 0;
};

template<typename Object, typename Data>
class CallbackMethod: public CallbackInterface<Data>
{
public:
    typedef void (Object::*PHandler)(const Data&);
    CallbackMethod(Object* obj, PHandler handler) : pObj(obj), pHandler(handler) {}
    virtual void run(const Data& data) { (pObj->*pHandler)(data); }
    virtual CallbackInterface* clone() const {return new CallbackMethod(*this); }

protected:
    Object* pObj;
    PHandler pHandler;
};

template <class Data>
class Callback
{
public:
    template <class Object>
    Callback(Object* obj, void (Object::*method)(const Data&) ):
        cb(new CallbackMethod<Object, Data>(obj, method)) 
    {
    }

    Callback(const Callback& other): cb(other.cb->clone() )
    {

    }

    Callback& operator=(const Callback& other) 
    {
        delete cb;
        cb = other.cb->clone();
        return *this;
    }

    ~Callback() 
    {
        delete cb; 
    }

    void operator()(const Data& data) const
    {
       cb->run(data);
    }

private:
    CallbackInterface<Data>* cb;
};

Example usage:

struct Foo
{
    void f(const int& x)
    {
        cout << "Foo: " << x << endl;
    }
};

struct Bar
{
    void f(const int& x)
    {
        cout << "Bar: " << x << endl;
    }
};

int main()
{
    Foo f;
    Callback<int> cb(&f, &Foo::f);
    cb(123); // outputs Foo: 123

    Bar b;
    cb = Callback<int>(&b, &Bar::f);
    cb(456); // outputs Bar: 456
}

As you can see, the Callback object itself does not require the object type to be passed in as a template argument, thereby allowing it to point to methods of any type provided that the method conforms to the signature: void some_class::some_method(const Data&). Store a list of these Callback objects in a class capable of calling all of them and you have yourself a signal with connected slots.

Thank you for your advice, but some of your assumptions are incorrect. I presented a simplified version that illustrated the particular problem I had. In reality, The Callback class inherits from an existing one that is specifically used to hold the data, and a pointer to the Object, for async remote call chains.Basically, you send a message and attach the Callback object. When you get a reply, the Callback object is recovered so you have access to the sending object and the state info. It also uses functionality from the Object class hierarchy. Still, I'll study your solution.
Alex O
@Alex: the solution should still be adaptable to your specific needs. The basic idea is to combine dynamic polymorphism and static polymorphism to allow you to have a single callback type that you can use for any purpose. You can still adapt it to inherit from a data base class but I recommend that you separate the data from the callback holder and have a sender that stores the data along with the Callback object. This way the data class can be inherited as much as you want without touching Callback which can be very general and store class methods to any class.
If the data/sender passed is always of the same type, then you can even eliminate the template <class Data> from Callback and make it a regular class instead of a class template. It can always pass this Data object you are currently inheriting from along with the sender. Think of the solution here as a building-block to allow you to easily solve the problem rather than a direct solution.
The data is free-form, the Object is inherited from a base class and the message passing interface is dictated by the framework: post(Message, BaseCallbackClass). The BaseCallbackClass does not provide a lot of functionality, just a pure virtual to be called on reply. Everybody subclass it to store data, invoke methods on the Object etc. I wanted to create a helper template to eliminate the repetitive work. I got it down to 2 lines: typedef GenericCallback<MyObject,MyDataType> MyCallback; and the call: post(message, new MyCallback(this, SomeMessageRelatedData,
Alex O
@Alex nice work! That is kind of the route I was suggesting, but I added a extra container kind of class so that the client doesn't have to write new MyCallback(...). You can make a wrapper which stores the base pointer with a constructor template that will instantiate the appropriate callback subclass (MyCallback in this case) and store it in the base pointer based on the arguments passed to the ctor template. It's a little wrapper to kind of hide away the memory management and template stuff, but may not be necessary for your case. It's basically how boost::function works.
"the client doesn't have to write new MyCallback(...)"The framework requires a heap allocated derivative of a BaseCallbackClass to be passed to the the post() function (to manage its lifetime), so the "new" is a must. Sorry, but I tried to omit the details to make the question more concise. I am not trying to be difficult, I just did not expect such a thorough optimization...
Alex O
Ah NP - I did not realize you could not modify BaseCallbackClass. Anyway, glad to see you found something that works for you!