C++: How to avoid vector from being destroyed after returning from method?

Question

I am trying to overload the division operator / for a Polynomial class, but the algorithm in itself is irrelevant. The problem is C++ seems to be corrupting my polynomial when I try to return it for some unknown reason.

This is the relevant part of the class:

class Polynomial
{
    public:
        .........
        Polynomial &operator/(const double &) const;
    private:
        int DEGREE;
        std::vector<double> poly_terms; // stores the polynomial coefficients
        .........
}

And this is the method that I can't get to work properly:

Polynomial &Polynomial::operator/(const double &rhs) const
{
    Polynomial result(10); //create new polynomial object with a maximum degree of 10
    double buffer;

    for(int i = 0; i <= DEGREE; i++)
    {
        buffer = poly_terms[i]; //poly_terms is of type vector<double>
        result.setCoeff(i, buffer / rhs); //this method assigns (buffer / rhs) to the i-th index of result's vector data-member.
    }
    return result; //return Polynomial instance
}

Right before the return clause is executed, a debugger shows that all data-members of the result object have their values correctly set as the algorithm was supposed to set them, including the vector data-member. So before return returns, result is 100% correctly built and thus the method's logic seems fine so far.

But everything monumentally fails right after the return statement is executed. For some reason the returned object gets its vector data-member changed to an empty vector (strangely, all other data-member that aren't objects, like DEGREE, are left fine as they were). I'm not sure if it's the same vector object that got emptied somehow, or if if it's a failed copy of the polynomial object which contains this vector object.

Does anyone know why this is happening and how I can avoid it?

UPDATE1: I should mention that I also tried creating the Polynomial object in this method by using new. And I also tried not returning a reference by removing the & so having a header like Polynomial Polynomial::operator/(const double &rhs) const. But both of these produced similar unwanted effects on the vector data-member.

UPDATE2: It seems I was able to track down the problem thanks to those who mentioned that I should not return a reference. The problem was this together with needing to overload the copy constructor and overloading the assignment operator to manually perform the copy of the vector data-member (not sure if both were needed, I just implemented all 3 of these things and now it works perfectly). Thanks to everyone for helping troubleshoot this.

Answer 1

Well, you return a reference to a local variable. Any code that will use the variable will be undefined behavior. There's a simple solution for your case: remove the &

// Notice that the function is not returning a reference anymore
Polynomial Polynomial::operator/(const double &rhs) const
{
    Polynomial result(10); //create new polynomial object with a maximum degree of 10
    double buffer;

    for(int i = 0; i <= DEGREE; i++)
    {
        buffer = poly_terms[i]; //poly_terms is of type vector<double>
        result.setCoeff(i, buffer / rhs); //this method assigns (buffer / rhs) to the i-th index of result's vector data-member.
    }
    return result; //return Polynomial
}

You might think it will slow your code because it copies the variable result. This is false. Your code will use copy elision. That means there's no copy made form the return statement.

Even if your compiler is dumb and won't use copy elision, it will use the move constructor of your class, and it's still much faster than a copy.

To read more about copy elision, check the documentation page here: Copy elision

Answer 2

At least in Visual studio this code should give you a warning, because you're using a reference to a temporary object that will be destroyed, so you're using a reference to youdontknowwhat. It's undefined behaviour.

You can simply return result by value in order to solve the problem. If the vector is big and you're worried about performance of returning by value, you can set a move constructor for Polynomial in which you move the vector instead than copying it.

Answer 3

I see three problems with your code (though not all of them are causing your troubles) :

1. As others pointed out, return the reference of an object that dies as soon as the program exits the operator overloading function. You can just return the Polynomial object

2. You loop with an index from 0 to DEGREE (so DEGREE + 1 items are modified). I assume DEGREE is something defined from the int input parameter of the Polynomial constructor. I would expect the loop to be defined from 0 to DEGREE - 1

3. I also would expect the "results" Polynomial object to be instantiated with DEGREE degrees instead of the hardcoded 10