current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 7 down vote favorite
2

In C++ (and C++11), classes defining a + and += operators often define a - and -= operators that do nearly the same thing (except + is replaced with - in the function).

What is the best way to avoid duplicated code here (and still achieve good performance)? I am sure there is a good way to do it with functors:

Here's what I've tried:

#include <iostream>
#include <functional>

struct num {
  int val;

  const num & operator +=(const num & rhs) {
    return op_equals<std::plus<int> >(rhs);
  }
  const num & operator -=(const num & rhs) {
    return op_equals<std::minus<int> >(rhs);
  }

  template <typename op>
  const num & op_equals(const num & rhs) {
    op operation;
    val = operation(val, rhs.val);
    return *this;
  }
};

int main() {
  num x{1};
  num y{2};
  x += y;

  std::cout << x.val << std::endl;
  return 0;
}

Note that this is a simple example; in reality the code in the += and -= operators is more complex.

Also, I am not married to functors in the solution-- any suggestions or advice?

share|improve this question

migrated from stackoverflow.com Jun 28 '12 at 2:12

This question came from our site for professional and enthusiast programmers.
1  
Have you looked at Boost.Operators? –  ildjarn Jun 27 '12 at 21:52
2  
Is replacing duplicated code with more code in total a win? –  Jon Jun 27 '12 at 21:52
4  
A common pattern is to implement operator+ in terms of operator+=, which minimises repeated code. –  Oli Charlesworth Jun 27 '12 at 21:53
2  
@Oliver: So using Boost is bad practice? –  Niklas B. Jun 27 '12 at 21:53
6  
I think in this particular case, KISS is the best practice. –  Stephen Chu Jun 27 '12 at 21:54

1 Answer 1

up vote 3 down vote

I actually quite like this pattern and use it relatively often. I'm not sure if there is something better than functors for this - if there is, I'm not really aware of it. That being said, with C++11, I'd write this in a slightly different way:

struct num {
  int val;

  const num& operator +=(const num & rhs) {
    return op_equals(rhs, std::plus<int>());
  }

  const num& operator -=(const num & rhs) {
    return op_equals(rhs, std::minus<int>());
  }

  template <typename op>
  const num & op_equals(const num & rhs, op&& o) {
    val = o(val, rhs.val);
    return *this;
  }
};

Note the rvalue reference (which is actually a "universal reference", so can bind to either an lvalue or rvalue reference), and the movement of op to a parameter - I don't really like having to specify it as a template parameter which then gets instantiated and called in the function. I think this way is slightly cleaner.

share|improve this answer
    
+1 Nice suggestions. I know to some people this feels like overkill, but to me it feels elegant (no duplicated code = no finding errors twice...) –  Oliver Feb 21 '13 at 16:03

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.