2

I've written a templated class that implements some basic operator overloading, following the guidelines provided by this particularly insightful answer:

template <typename _type>
class myClass {
    // ...
    template<_input_type> myClass<_type>& operator+=(const myClass<_input_type>& _other);
    // ...
}

with the arithmetic compound operators written as members:

template <typename _type>
template <typename _input_type>
myClass<_type>& myClass<_type>::operator+=(const myClass<_input_type>& _other) { 
    static_assert(std::is_arithmetic<_type>::value);
    // do stuff
    return *this;
};

and the non-compound operator as a non-member:

template <typename _type, typename _input_type> 
inline myClass<_type> operator+(myClass<_type>& _o1, myClass<_input_type> _o2) { 
    return _o1+=_o2;
};

However, due to the template myClass can be used for several data types, some of them non-numeric that can't handle +,-,* or / operators, and as such I was wondering what are the downsides of implementing all operator overloading code as non-member functions, so that e.g. I could just placed them all on a separate header file that would only need to be included if there is need for arithmetic functionality. I understand one solution would be to define a new class myNumericClass : public myClass that just implements operator overloading, but that would require a new typename and limit the versatility of myClass.

joaocandre
  • 1,621
  • 4
  • 25
  • 42
  • 1
    There is no obvious problem with having the operators in another header. It's a good solution IMHO. The inheritance solution is not good. – Matthieu Brucher Oct 25 '18 at 13:59

1 Answers1

4

The primary shortcoming of implementing compound assignment as a non-member is inconsistency with the simple (copy or move) assignment operator. A simple copy or move assignment (i.e., operator=) must be implemented as a member function, or the compiler will outright reject the code.

Given that copy/move assignment must be implemented as member functions, many prefer to implement compound assignment as members as well.

As an aside, this code:

template <typename _type, typename _input_type> 
inline myClass<_type> operator+(myClass<_type>& _o1, myClass<_input_type> _o2) { 
    return _o1+=_o2;
};

...is, IMO, highly inadvisable. The general style is fine, but you've mixed up which operand to pass by value and which to pass by reference. As a result, it may be needlessly inefficient, and (much worse) modifies its left operand, so it really acts like += instead of +. What you almost certainly want is more like this:

template <typename _type, typename _input_type> 
inline myClass<_type> operator+(myClass<_type> _o1, myClass<_input_type> const &_o2)
{ 
    return _o1+=_o2;
};

Here we pass the left operand by value, so when the function is called a temporary value is created and initialized from the left operand. We then modify that temporary value (without changing the original) and return it. Since we do return it, there will be copy elision (optional on older compilers, but mandatory since C++17) which means it'll normally really just be a reference to the destination, so effectively, something like: a = b + c; will be treated as: a = b; a += c;. Since we only need the previous value of the right operand, we pass it as a reference to const to avoid an unnecessary copy (though, depending on the type, passing by reference may not gain enough to care about, or could even be a loss. But it can be a big gain, and is rarely more than a tiny loss).

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • I have implemented copy & move assignment operators, just left it out of the example because they did not seem relevant. Also, there was a typo on the code, I was actually passing both arguments as non-const references, but evidently your point still stands and I'll take it into account. Still, wouldn't it be better to pass *both* operands as const references, won't pass any of them by value create an unnecessary intermediary copy? – joaocandre Oct 25 '18 at 14:37
  • @joaocandre if you need to make a copy anyhow then pass by value is fine – 463035818_is_not_an_ai Oct 25 '18 at 14:42