It’s impossible to tell what’s wrong without a Minimal, Complete, Verifiable Example. In this case, that means the relevant parts of your class definition and a main() function.
In case this helps, here’s an example of three different ways to write an efficient operator+ and an operator-. They use some fairly advanced techniques. The implementation of operator+ relies on the compiler to use the provided move constructor and optimize away any redundant copies. The implementation of operator- calls a programmatic constructor that uses a dummy parameter for the unary version, then a function object for the binary version.
#include <array>
#include <cassert>
#include <cstdlib>
#include <functional>
#include <iostream>
#include <new>
#include <utility>
#include <vector>
using std::cout;
using std::endl;
using std::size_t;
// A four-element vector, implemented internally with std::vector.
class vec4i {
private:
static constexpr size_t size = 4U;
/* Object holding the vector data. The compiler knows how to destroy it,
* so we do not need to write our own destructor.
*/
std::vector<int> data;
/* Empty class used as a dummy parameter to tell the compiler that we are
* calling the programmatic constructor, not the copy constructor.
*/
struct op_opposite {};
// Programmatic constructor used by unary operator-:
vec4i( const vec4i& v, const op_opposite& /* unused */ )
: vec4i()
{
for (size_t i = 0; i < size; ++i)
data[i] = -v[i];
}
/* The type of a generic function object implementing a binary function. */
using binary_fun = std::function<int(const int, const int)>;
/* We're going to demonstrate a different approach for operator-, a
* programmatic constructor using an arbitrary binary function object from
* <functional>.
*/
vec4i( const vec4i& u, const vec4i& v, const binary_fun f )
: vec4i()
{
for ( size_t i = 0; i < size; ++i )
data[i] = f(u[i], v[i]);
}
public:
// Default constructor initializes matrix to zeroes:
vec4i() : data(size) {}
// Constructors that provide values:
vec4i( const std::vector<int>& v ) : data(v) {}
vec4i( const int x, const int y, const int z, const int w )
: data{x,y,z,w}
{}
vec4i( const int v[size] )
: data(size)
{
for ( size_t i = 0; i < size; ++i )
data[i] = v[i];
}
// Copy constructor that makes a deep copy:
vec4i(const vec4i& v) : data(v.data) {}
// Move constructor that avoids a deep copy:
vec4i(vec4i&& v) : data(std::move(v.data))
{}
// One final example, using STL iterators, since it's a one-liner:
vec4i(const std::array<int, size>& a) : data(a.begin(), a.end()) {}
/* Provide an assignment operator from any object that can be used to
* construct this type. Ensure that the class does not assign from its own
* internal data.
*/
template<class T> vec4i& operator=(const T& v)
{
// Check for self-assignment.
if ((void*)this != (void*)&v) {
// Call the destructor, then create a new object in-place.
this->~vec4i();
new(this) vec4i(v);
}
return *this;
}
vec4i operator=(vec4i&& v)
// A more efficient assignment that moves rather than copies the data:
{
if ((void*)this != (void*)&v) {
data.clear();
data.swap(v.data); // Could also write: data = std::move(v.data);
}
return *this;
}
// Accessor returning a rvalue:
int operator[]( const size_t i ) const
{
return data[i];
}
// Accessor returning a lvalue:
int& operator[]( const size_t i )
{
return data[i];
}
// The implementation of operator+ that this example is demonstrating:
vec4i operator+(const vec4i& v) const
{
vec4i temp;
for ( size_t i = 0; i < size; ++i )
temp[i] = data[i] + v[i];
/* Since we provided a move constructor and move assignment above, the
* compiler should be able to optimize away this copy!
*/
return temp;
}
// Returns a vector with the sign of each elemeent flipped:
vec4i operator-(void) const
{
// The dummy second parameter selects the programmatic constructor.
return vec4i( *this, op_opposite() );
}
// Now we use the constructor above to construct the return value:
vec4i operator-(const vec4i& v) const
{
// Function object wrapping a-b, initialized once:
static const binary_fun subtract = std::minus<int>();
// Create the returned temporary object in place.
return vec4i( *this, v, subtract );
}
};
std::ostream& operator<<( std::ostream& os, const vec4i& v )
// Serialize our vector.
{
return os <<
'<' << v[0] << ',' << v[1] << ',' << v[2] << ',' << v[3] << '>';
}
int main(void)
{
const vec4i u = {0,0,0,1}, v = {1,0,0,0};
// Output: "<1,0,0,1>"
cout << u+v << endl;
// Output: "<-1,0,0,1>"
cout << u-v << endl;
// output: "<-1,0,0,1>"
cout << -(v-u) << endl;
return EXIT_SUCCESS;
}
