callback vs lambda

Question

Suppose I have the following code that I wish to refactor:

int toFuture()
{
  precalc();
  int calc = 5 * foobar_x() + 3;
  postcalc();
  return calc;
}

int toPast()
{
  precalc();
  int calc = 5 * foobar_y() - 9;
  postcalc();
  return calc;
}

In classic-C, I would refactor this code into a worker() which accepts a function pointer that does the calculation: common code in worker(), specific code provided by function pointer.

With C++11, should I be using a lambda instead? If so, how would I implement it, in this case?

Edit: it just crossed my mind that a template may also work. How would a template implementation compare against the other two?

With just two lines of repeated code, any refactoring is going to have a hard time getting any simpler. The simplest thing would be to put the pre and post steps into a RAII style class but without further use, even that is going to increase a strict lines of code cost. — CB Bailey, Oct 19 '11 at 22:05
For the sake of clarity, I have simplified the problem in the example above. But you are right: in real code, one needs to weigh the clarity of code written vs other factors. — kfmfe04, Oct 20 '11 at 01:14
OK, so what exactly are you trying to get out of this refactor? Usually library code should be written to make the client code as simple and easy as possible and there isn't really much you can do to simplify a straight function call such as `toPast`. Most of the answers make the client code more complex or at least less obvious. — CB Bailey, Oct 20 '11 at 20:48
I am trying to avoid duplicate code. I always assume that code will change or have to be debugged in the future. In my code above, I have to make changes in TWO places, if the common, surrounding code changes. That qualifies as a "bad smell" for me. ildjarn has the best approach, but taking your point into account, I can easily wrap his direct calls with toPast() and toFuture() - an easy, small modification. — kfmfe04, Oct 21 '11 at 02:46

ildjarn · Accepted Answer · 2012-03-14T21:06:25.783

41

One approach:

template<typename CalcFuncT>
int perform_calc(CalcFuncT&& calcfunc)
{
    precalc();
    int const calc = std::forward<CalcFuncT>(calcfunc)();
    postcalc();
    return calc;
}

int main()
{
    perform_calc([]{ return 5 * foobar_x() + 3; }); // toFuture
    perform_calc([]{ return 5 * foobar_y() - 9; }); // toPast
}

edited Mar 14 '12 at 21:06

answered Oct 19 '11 at 22:06

ildjarn

62,044
9
127
211

aye - for those who are not yet up-to-par on rvalue references, here is a link: http://stackoverflow.com/questions/5481539/what-does-t-mean-in-c0x – kfmfe04 Oct 21 '11 at 02:49

Jason · Answer 2 · 2011-10-20T02:33:53.020

22

If you are wanting a template approach using C++11 features, that could look as simple as:

template<typename FuncType>
auto calculation(FuncType&& func) -> decltype(func())
{
    precalc();
    auto ret = func();
    postcalc();
    return ret;
}

You would then simply call your calculation function and pass it either a lambda, a functor, or a function-pointer. Your only souce of difficulty in this instance would be if you passed a function that had a void return-type ... in that case you will get a compiler error (which is a good thing vs. a runtime error).

edited Oct 20 '11 at 02:33

answered Oct 19 '11 at 22:29

Jason

31,834
7
59
78

aye - nice use of auto, decltype – kfmfe04 Oct 19 '11 at 22:54
1

Passing `func` by non-const reference would disallow temporary functors **and lambdas** from being passed in. Pass by value instead, like the standard library does. – ildjarn Oct 19 '11 at 23:28
BTW, I fixed this version per your suggestion, but would it be good to have two overloaded versions, one with a const-reference argument, and one with a non-const reference argument? The reason is that copy-by-value would not capture the side-effects of a functor-object that you would want to pass into the function, and retain the new "state" of that functor for later use (i.e., the function call modifies the internal data of the functor, and you then want to maintain that internal data for another use of the functor somewhere else). – Jason Oct 20 '11 at 01:51
2

@Jason : That's an option, but typically a functor should not own its state anyway -- the state should be owned externally and held by reference/pointer inside of the functor. That said, since we're already using C++11 syntax, if `func` were of type `FuncType&&` then both would be supported with the same code simultaneously, since reference collapsing rules will treat an lvalue passed in as a `FuncType&` anyway. I like this enough that I made the change in my answer as well. :-] – ildjarn Oct 20 '11 at 01:57

score 8 · Answer 3 · answered Oct 20 '11 at 08:31

8

I'd say you're refactoring from the wrong side:

struct CalcGuard {
  CalcGuard() { /* replaces precalc() */ }
  ~CalcGuard() { /* replaces postcalc() */ }
};

int toFuture()
{
  return CalcGuard(), calc = 5 * foobar_x() + 3;
}

int toPast()
{
  return CalcGuard(), calc = 5 * foobar_y() - 9;
}

answered Oct 20 '11 at 08:31

MSalters

173,980
10
155
350

Interesting implementation! But you got me scratching my head a bit here... ...it's definitely a creative solution. I can understand that you are taking advantage of the lifetimes of variables here in conjunction with the lhs to rhs order of expression evaluation for comma-separated expressions. IMHO, this kind of code is a good implementation for a test question, but not quite as good for refactoring production code. – kfmfe04 Oct 20 '11 at 09:23
@kfmfe04: If you think it's too fancy, a named variable would work equally well. Since they're one-liners, expression scope and function scope are practically equal. – MSalters Oct 20 '11 at 10:07
2

+1, but the main disadvantage this has is that if `postcalc` had any possibility to `throw`, then the exception handling that one would be forced to put inside of `~CalcGuard` would be in the wrong place. – ildjarn Oct 20 '11 at 12:59
`postcalc()` throwing is a nasty case regardsless. You'd have the same problem if `postcalc()` was in a `catch(...)`/rethrow block, and else it wouldn't be executed in case of a normal exception. – MSalters Oct 20 '11 at 15:13

score 1 · Answer 4 · answered Oct 19 '11 at 22:20

There is a C/C++ way to do this, and a C++11 way. Neither way involves lambdas or templates.

The C/C++ way:

double MyFunc (int x, float y) { return x + y ; }

int main()
  {
  double (*pf) (int, float) ;
  pf = MyFunc ;
  pf (101, 202.0) ;
  }

The C++11 way:

#include <functional>

double MyFunc (int x, float y) { return x + y ; }

int main()
  {
  std::function<double (int, float)> f ;
  f = MyFunc ;
  f (51, 52.0) ;
  }

In either case, you just pass pf or f to your refactored function as a parameter. Using lambdas or templates is overkill here.

`std::function<>` has more runtime overhead caused by type erasure, yet lambdas and templates are overkill..? — ildjarn, Oct 19 '11 at 22:26

callback vs lambda

4 Answers4

Linked