Is returning static variables in methods a bad idea?

Question

I've searched around and can't seem to find an answer to my question. I am doing a project where I have to redefine some operators (+, -, *, etc.) for operations between vectors and polynomials. As far as I know, those operators are supposed to return copies of the objects so as to not modify them directly if we're simply calling the operator (ie vect1+vect2; instead of vect1 += vect2;) and not placing the result anywhere.

Now, I've seen all around that using static variables is a bad practice, but how can I avoid doing that when a method needs to return a copy of the result instead of modifying the object?

The questions here and here didn't really help because they don't address my particular issue.

Here is an example of what I mean:

template <class elem>
Vect_variable<elem>& Vect_variable<elem>::operator+(const Vect_variable& operand)
{
    if (taille >= operand.taille)
        {
        static Vect_variable<elem> temp_v;
        temp_v = *this;
        for (std::size_t i = 0; i<operand.taille; i++)   
            {
            temp_v[i] += operand.vecteur[i];
            }
        return temp_v;
        }
    else
        {
        static Vect_variable<elem> temp_v;
        temp_v = operand;
        for(std::size_t i = 0; i<taille; i++)
            {
            temp_v[i] += vecteur[i];
            }
        return temp_v;
        }
}

In this case you can see that I am creating static Vect_variable for the temporary variable used. Is there any way to do this otherwise?

Answer 1

The robust and simple method is to return a (non static) local variable by value. In some cases that is also the efficient method.

If you care a lot about efficiency, then you want an alternate execution path for the cases where one of the two inputs is an rvalue. In that case, you don't want a local variable at all. Instead you want to take an input by rvalue reference, modify it and return it by rvalue reference.

To avoid an excess of duplicate code, you probably want to define an operator+= and put the real work in that method and have all versions of operator+ delegate the work to operator+=

There are situations in which returning a static variable by reference is more efficient than all the above. But there aren't a lot of such situations and they are filled with enough hazards that you shouldn't make that choice.

Having seen the comments you made on the earlier answer: You are correct that returning a static by reference avoids the big problem in having a polymorphic return from an override of a virtual function. But it does so at the expense of creating its own problems. For a trivial example, consider someone using your + to compute A+(B+C). Don't ask why they put in those inappropriate (). Just realize they had no way to know they shouldn't. So your operator copies B to a static and adds C to that, then it copies A to the same static and adds the static to that and returns 2A. While that example is contrived, it is just one of many things you might do with + that go horrible wrong if it returns a reference to a static.

Answer 2

Yes. Don't make the variable static. That way each call to the function will get its own fresh new variable, and you return that.

Also, it's automatic (on the stack) rather than static, you can declare the variable and initialize it in one go.

... and I hadn't noticed you are returning a reference to the vector. Don't do that. Return it by value.

template <class elem>
Vect_variable<elem> Vect_variable<elem>::operator+(const Vect_variable& operand)
{
    if (taille >= operand.taille)
        {
        Vect_variable<elem> temp_v = *this;
        for (std::size_t i = 0; i<operand.taille; i++)   
            {
            temp_v[i] += operand.vecteur[i];
            }
        return temp_v;
        }
    else
        {
        Vect_variable<elem> temp_v = operand;
        for(std::size_t i = 0; i<taille; i++)
            {
            temp_v[i] += vecteur[i];
            }
        return temp_v;
        }
}

The next thing to do, is to notice that you are doing almost the same thing in both branches. It just depends on which taille is shorter. So use a pair of pointers:

{
    decltype(this) one, two;
    if (taille >= operand.taille)
        {
        one = this;
        two = &operand;
        }
    else
        {
        one = &operand;
        two = this;
    }

    Vect_variable<elem> temp_v = *one;
    for (std::size_t i = 0; i<two->taille; i++)   
        {
        temp_v[i] += two->vecteur[i];
        }
    return temp_v;
}

The final comment is that it is usually good practise to write operator += first, and then write operator + as a non-member binary function with signature:

TYPE operator +(TYPE lhs, const TYPE& rhs)
{
    lhs += rhs;
    return lhs;
}

Note that lhs is taken by value (so you already have your own copy of it). The advantage of the non-member function is that if you have any conversion functions to TYPE, they will operate symmetrically on the left- and right- hand side.