1

I've been scratching my head all day new to operator overloading and my operator+ function computes the data correctly, but I need to pass the data of temp class to operator= to assign it to a separate instance of my class and returning temp does not work (I imagine the data is being destroyed on exit?) the whole idea is from main x = y + z is called to add the data of two vectors from y and z and asign it to x and I get the computation of y + z correctly, but passing it to x I've hit a brick wall what is wrong? or does anyone have idea? This is piece my code from my class

VecXd& operator=(const VecXd& rhs)
{
    VecXd<V> temp;
    temp.dimension = rhs.dimension;
    cout << "operator= check dimension is..." << temp.dimension << endl;

    for(int i = 0; i < rhs.dimension; i++)  //Passing data to x?
    {
        cout << "test" << endl;
        temp.vecArr[i] = rhs.vecArr[i];
        cout << temp.vecArr[i] << " our new value" << endl;
    }
}

friend VecXd& operator+(VecXd& lhs, VecXd& rhs){

    VecXd<V> temp;
    cout << lhs.dimension << "-lhs d-" << rhs.dimension << "-rhs d-" << endl; //works
    if(lhs.dimension == rhs.dimension) //dimension level check
    {
        temp.vecArr = new V[lhs.dimension];
        for(int i = 0; i < lhs.dimension; i++)
        {
            temp.vecArr[i] = lhs.vecArr[i] + rhs.vecArr[i];
            cout << temp.vecArr[i] << " our new value" << endl;
        }
        //return *temp.vecArr;
        return temp; //***? how to pass data?
    }
    else{
        cout << "Dimensions do not match!!! Error!" << endl;
    }
}

any idea? don't be to harsh... haha..... :l

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
SxMZ
  • 147
  • 3
  • 13
  • 1
    `operator+` should be in terms of `operator+=`. – chris Sep 18 '13 at 23:26
  • I have += for separate operation, doing what you said leaves the compiler asking for operator+ and operator= by itself :/ – SxMZ Sep 18 '13 at 23:33
  • 1
    See [operator overloading](http://stackoverflow.com/questions/4421706/operator-overloading) for what I mean. – chris Sep 18 '13 at 23:38

2 Answers2

1

Your assignment operator is bogus: the operation the assignment is supposed to do is to make the object pointed to by this the same as the object on the right hand side. Your assignment operator creates a temporary object and sets this object up. However, it goes away when exiting the assignment operator. Also, you declared your assignment operator to return a reference which conventionally returns a reference to *this but there is no return statement.

Unless I have a very good reason to do it differently, I implement my assignment operator in terms of the copy constructor, the destructor, and a swap() function:

VecXd& VecXd::operator=(VecXd rhs) 
{
    this->swap(rhs);
    return *this;
}

The copy constructor is called to create the argument to function, nicely copying the object. Then the newly created copy is exchanged with the content of the object being assigned to and, after returning *this the original content is released by the destructor of the temporary object created for rhs. All what is required is a relatively function swap():

void VecXd::swap(VecXd& other)
{
     std::swap(this->dimension, other.dimension);
     std::swap(this->vecArr, other.vecArr); 
}

The swap() function assumes that the two members can be swapped, of course. Since I haven't seen your declaration of VecXd I can't tell whether this would work but it should generally work.

I haven't really looked at your addition operator.

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • thank you Dietmar for your information, but I'm still having bit trouble... I'm working with template classes and have implemented the swap function and operator=, but am getting a no match for operator= issue and the only thing that seemed to resolve that is adding const to operator=(const VecXd& rhs) but that creates issue for swap can you help? – SxMZ Sep 19 '13 at 01:14
0

This Caltech C++ Operator Overloading Page should have everything you need and a relatively clear explanation of the logic involved. It is not by any means required that you follow the rules and guidelines laid out there, but they work well in the general case.

In the specific case of the arithmetic operators, the operation is often defined in terms of the += operator, as noted in the comment on your original post by chris. This means that you typically pass in the right and left arguments, make a copy of lhs, and find the result by using lhs_copy += rhs; it's easier to avoid code duplication with this in mind; you'll only be creating the addition logic once in += and then calling it in +.

The result you return cannot be a reference - nothing created in the function will persist, but neither the lhs or rhs arguments should be changed. Instead, you simply return the duplicate lhs you incremented as an instance - in your case the function signature is:

const VecXd operator+(const VecXd& lhs, const VecXd& rhs). 

Your input can and should be const since neither item will be changed in the function, and the Caltech page recommends the return type be a const instance to disallow certain odd operations, which again is good practice in many cases.

  • 2
    The potential side effects of allowing temp-variable modification post-return is arguably *far* outweighed by the benefit of move-semantics in C++11 and beyond. – WhozCraig Sep 18 '13 at 23:50
  • A quote from the Caltech site: "YOU MUST CHECK FOR SELF-ASSIGNMENT!" That shows ignorance: any assignment operator which actually depends on a check for self-assignment (i.e., removing the check renders the logic wrong for self-assignments) has a near 100% chance of not being exception safe! Likewise, returning a the result by `const` disables the possibility of the result to be moved from which is a silly idea. Although it would render the interface slightly asymmetric, I would also take the left-hand side argument by value to potentially support copy elision on the temporary value. – Dietmar Kühl Sep 19 '13 at 00:17