1

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?

Community
  • 1
  • 1
  • 3
    Simple answer: stop returning a reference. _"As far as I know, those operators are supposed to return copies of the objects"_ Yes, exactly, so why are you returning a reference? That's not a copy of an object. If you stop incorrectly returning a reference then you don't need to use `static`. – Jonathan Wakely Dec 16 '15 at 13:36
  • @JonathanWakely Yes, unfortunately I am defining a pure virtual method from a class I inherited this from, and that's why I have to return references. – DefinitelyNotMe Dec 16 '15 at 16:53
  • 1
    Then the base class is broken and dumb. You need to check what the expected behaviour is, maybe it is supposed to modify `*this`. Returning a reference in other situations makes no sense, what is it supposed to a reference to? It can't be a static variable, because that's even more broken. – Jonathan Wakely Dec 16 '15 at 17:41
  • @JonathanWakely That was my first reaction as well. It is specified that I have to inherit from that base class, but the class itself serves no purpose whatsoever. Pure virtual operators make no sense to me because you are returning copies, not references. No choice though :( – DefinitelyNotMe Dec 16 '15 at 18:07

2 Answers2

4

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.

  • It doesn't work, since it says that it cannot return a reference to an object that has gone out of scope. Perhaps changing the return type? And also, declaring and initializing at the same time didn't work work for me (some random numbers appearing here and there, and not doing it fixed it for me) Edit - Alright I'll change that :) Thank you – DefinitelyNotMe Dec 16 '15 at 12:35
  • And by the way do you have any idea why declaring and defining on the same line would give unexpected results ? – DefinitelyNotMe Dec 16 '15 at 12:41
  • 3
    If you have a static variable, then declaring and initializing on the same line will have very surprising effects. The *first* time you call the function, the variable will be initialized; second and subsequent calls will retain the value on the previous call. Just don't do that! – Martin Bonner supports Monica Dec 16 '15 at 12:46
  • Oh, and if the given method is a redefinition of an inherited method, is there still a way to overload the given method without it returning a reference? I was lead to believe inherited methods could only return references if one wanted to overload them. – DefinitelyNotMe Dec 16 '15 at 12:46
  • 2
    Do you really mean "redefinition", or do you mean "override of a virtual function"? In the latter case, you must return the same thing as the base class returns, or if that is `Base*` or `Base&` you can return `Derived*` or `Derived&`. If that is what is going on here, you need to reconsider the design! – Martin Bonner supports Monica Dec 16 '15 at 12:56
  • Better is a friend operator that takes its lhs by value, and modifies it. Easiest to implement via `+=`. – Yakk - Adam Nevraumont Dec 16 '15 at 13:24
  • @Yakk : Doesn't actually need to be a friend - and my last code snippet shows exactly that (right down to lhs by value!) – Martin Bonner supports Monica Dec 19 '15 at 06:21
2

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.

JSF
  • 5,281
  • 1
  • 13
  • 20
  • I've actually done it the other way around ^^'. My += uses the code from my +. – DefinitelyNotMe Dec 16 '15 at 12:51
  • 1
    If you don't care about efficiency, having += delegate to + can make simpler looking code. In many cases the efficiency doesn't matter. Maybe in some obscure cases the efficiency difference is reversed. But as a general habit, it is better to have + delegate to += because when the efficiency difference is big, that is the more efficient approach. – JSF Dec 16 '15 at 12:53
  • That's a *much* better description of the design issues than my answer. – Martin Bonner supports Monica Dec 19 '15 at 06:23