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

Fast templated call back implementation

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

Problem

Below is the code for my templated callback implementation. Currently it works for a single parameter. One thing I was going to try and do next was increase the argument to the function from 1..N arguments. Ideally this would be as simple as adding Args... everywhere using variadic templates. I don't have a lot of experience yet with variadic templates so any advice on this is helpful.

#include 
#include 

template 
class Callback
{
  public:
  typedef R (*FuncType)(void*, Arg);
  Callback (FuncType f, void* subscription) : f_(f), subscription_(subscription) { }

  R operator()(Arg a)
  {
    (*f_)(subscription_,a);
  }

  private:
  FuncType f_;
  void* subscription_;
};

template  
R CallbackWrapper (void* o, Arg a)  
{
  return (static_cast(o)->*Func)(a);
}

class Pricer 
{
  public:
  typedef Callback CbType;

  void attach ( CbType cb )
  {
    callbacks_.emplace_back(cb);
  }

  void receivePrice ( double price )
  {
    broadcast(static_cast(price*100));
  }

  void broadcast (unsigned int price)
  {
    for ( auto& i : callbacks_)
    {
      i(price);
    }
  }

  private:
  std::vector callbacks_;
};

class Strategy
{
  public:
   Strategy(Pricer*  p) :  p_(p) 
   { 
     p->attach(Callback(&CallbackWrapper, static_cast(this)));
   } 

   void update(unsigned int price)
   {
    //update model with price
     std::cout << "Received price: " << price / 100.0 <<  std::endl;
   }

  private:
  Pricer* p_;
};

int main ( int argc, char *argv[])
{
  Pricer p;
  Strategy s(&p);

  p.receivePrice(105.67);
}

Solution

The functionality you are trying to create already exists in std::function<> using std::bind<> to help.

Comments on the rest of the code:

Here you use emplace back:

void attach ( CbType cb )
  {
    callbacks_.emplace_back(cb);
  }


Emplace back is usually used when you have the arguments and want to build the object in place using the arguments. By passing the object you are going to just invoke the copy constructor. As a result there is no benefit from using it. Though there is nothing wrong with using it either. Currently I am still working out when to use emplace_back() over push_back() but this is one situation where I would still use push_back().

Also because you pass the argument by value you are copying the object into the function then using the copy constructor to put it in the array resulting in another copy. So here I would pass by reference.

void attach(CbType const& cb)
  {
    callbacks_.push_back(cb);
  }


Don't use unnecessary casts

broadcast(static_cast(price*100));

// This is easier to read as:

broadcast(price*100); // double is auto converted to unsigned int


Use standard types:

typedef Callback CbType;

// Replace with:

typedef std::function CbType;


If you use the standard function then the equivalent to creation becomes much simpler

p.attach(std::bind(&Strategy::update,this, _1));


Don't use pointers where a reference is a better choice:

Strategy(Pricer*  p) :  p_(p)


You are passing a pointer and not checking for NULL. Actually the code never checks for NULL so you must use a valid pointer. In this case you may as well pass a reference. If you want to store this internally as a pointer then take its address inside your object.

Strategy(Pricer&  p) :  p_(&p)  // p can never be invalid.

Code Snippets

void attach ( CbType cb )
  {
    callbacks_.emplace_back(cb);
  }
void attach(CbType const& cb)
  {
    callbacks_.push_back(cb);
  }
broadcast(static_cast<unsigned int>(price*100));

// This is easier to read as:

broadcast(price*100); // double is auto converted to unsigned int
typedef Callback<void,unsigned int> CbType;

// Replace with:

typedef std::function<void(unsigned int)> CbType;
p.attach(std::bind(&Strategy::update,this, _1));

Context

StackExchange Code Review Q#31128, answer score: 4

Revisions (0)

No revisions yet.