2
#include <iostream>
using namespace std;
class A
{
public:
    A(int a)
    {
        length = a;
    }
    ~A(){}

    friend A operator +(A& var1, A& var2);
    A& operator=(A &other);

    int length;
};

A operator +(A& var1, A& var2)
{
    return A(var1.length + var2.length);
}

A& A::operator=(A &other)
{
    length = other.length;
    return *this;
}


int main()
{
    A a(1);
    A b(2);
    A c(3);
    c = a;   // work
    c = a + b;  // does not work
    cout << c.length ;
    return 0;
}

In main(), c = a is successfully compiled but "c = a + b" is not. However, in A& A::operator=(A &other), if I change (A &other) into (A other) then it works. Can anyone help me with this case?

Mr Cold
  • 1,565
  • 2
  • 19
  • 29

2 Answers2

8

The simplest fix is to make your assignment overload take it's parameter by const reference.

Then the temporary returned by a + b can be used with it.

A& A::operator=(A const & other)
{
    length = other.length;
    return *this;
}

You'll probably want to do the same thing with your operator+ so that c = a + a + a; will work as well.

Bill Lynch
  • 80,138
  • 16
  • 128
  • 173
  • Thank you for your answer. However, I still don't understand why your way works. As I know, a + b will return an object A, so what is the difference between not using "const" and using "const" in A& A::operator=(A const & other)? By the way, I've never seen "const" being used after the class name like this. Can your explain it for me? Thank you. – Mr Cold Mar 14 '15 at 04:26
  • @MrCold: There's no difference between `A const&` and `const A&`. Similarly, `A const*` is the same as `const A*`. But `A* const` is a different beast completely. – Ben Voigt Mar 14 '15 at 04:31
  • 1
    @MrCold the difference is that C++ doesn't let you bind a non-`const` reference to a temporary such as the `A` returned by `a + b`. Instead, if `operator=`'s argument is a `const` reference it *is* allowed to bind. The reason C++ prevents the non-`const` reference binding is just that it makes it easy for code to modify a value that's about to be destroyed - the changed state isn't then around to do anything with, so it's considered a "code smell" and usually indicates a programming mistake, though occasionally it'd be useful. – Tony Delroy Mar 14 '15 at 05:53
  • `A &A::operator=(A other)` has the advantage that it serves as copy-assignment and move-assignment in one. – M.M Mar 14 '15 at 06:19
  • @MattMcNabb And the disadvantage that it isn't that great an implementation for either. – T.C. Mar 14 '15 at 09:07
  • @TC can you elaborate? my understanding is that it is the optimal implementation (when using copy-swap) – M.M Mar 14 '15 at 09:57
  • @MattMcNabb (That didn't ping.) Copy-and-swap is usually inefficient because it doesn't reuse the lhs's resources. Howard Hinnant has written quite a bit about it. – T.C. Mar 28 '15 at 12:52
  • @T.C. OK, I eventually found [this](http://stackoverflow.com/questions/18303287/when-is-overloading-pass-by-reference-l-value-and-r-value-preferred-to-pass-by/18303787#18303787) under a deceptive title. – M.M Mar 29 '15 at 00:24
-1

The problem is that operator + returns a temporary object

friend A operator +(A& var1, A& var2);

But a temporary object may not be bound to a non-const reference that is the type of the parameter of the assignment operator.

A& operator=(A &other);

So the compiler issues an error for statement

c = a + b;

You have three possibility.

The first one os to declare the parameter of the copy assignment operator as constant reference

A& operator=(const A &other);

It is the simplest approach.

The second one is to declare a move assignment operator instead of the copy assignment operator. In this case you explicitly need to define also a copy or move constructor. In this case instead of

c = a;

you have to write

c = std::move( a );   // work

For example

#include <iostream>
using namespace std;
class A
{
public:
    A(int a)
    {
        length = a;
    }
    ~A(){}

    friend A operator +(A& var1, A& var2);
    A& operator=(A &&other);
    A( A && ) = default;
    int length;
};

A operator +(A& var1, A& var2)
{
    return A(var1.length + var2.length);
}
A& A::operator=(A &&other)
{
    length = other.length;
    return *this;
}

int main()
{
    A a(1);
    A b(2);
    A c(3);
    c = std::move( a );   // work
    c = a + b;  // does not work
    cout << c.length ;
    return 0;
}

And at last you could have the both operators simultaneously. For example

#include <iostream>
using namespace std;
class A
{
public:
    A(int a)
    {
        length = a;
    }
    ~A(){}

    friend A operator +(A& var1, A& var2);
    A& operator=(const A &other);
    A& operator=(A &&other);
    A( const A & ) = default;
    int length;
};

A operator +(A& var1, A& var2)
{
    return A(var1.length + var2.length);
}

A& A::operator=(const A &other)
{
    length = other.length;
    return *this;
}

A& A::operator=(A &&other)
{
    length = other.length;
    return *this;
}

int main()
{
    A a(1);
    A b(2);
    A c(3);
    c = a;   // work
    c = a + b;  // does not work
    cout << c.length ;
    return 0;
}

In this case in statement

    c = a;   // work

there will be called the copy assignment operator and in statement

    c = a + b;  // does not work

there will be called the move assignment operator.

Of course you also may have the copy constructor and move constructor simultaneously the same way as the copy assignment operator and the move assignment operator. For your class you could all them define as defaulted. For example

#include <iostream>
using namespace std;
class A
{
public:
    A(int a)
    {
        length = a;
    }
    ~A(){}

    friend A operator +(A& var1, A& var2);
    A& operator=(const A &other) = default;
    A& operator=(A &&other) = default;
    A( const A & ) = default;
    A( A && ) = default;
    int length;
};

A operator +(A& var1, A& var2)
{
    return A(var1.length + var2.length);
}


int main()
{
    A a(1);
    A b(2);
    A c(3);
    c = a;   // work
    c = a + b;  // does not work
    cout << c.length ;
    return 0;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • You're making it unnecessarily complicated by bringing "moving" into the picture. The OPs just trying to create a simple value type with no pointers - move isn't useful and may hide the problem but isn't a good fix. – Tony Delroy Mar 14 '15 at 05:46
  • @Tony D In my opinion you are entirely wrong I am providing a general observation of the problem. It is always better than some one solution. – Vlad from Moscow Mar 14 '15 at 06:12