Copy Constructor in c++ not working as expected

Question

I was trying to make a Complex class which would represent the Complex numbers in c++ but I have encountered an error. The copy constructor seems to be the problem but I am not sure as to what is wrong or why it is wrong. The / operator works well as I return a reference to an object of Complex class. but the * operator does not works when I return an object of Complex class. Please explain this behaviour,

edit: I have used the new operator and a custom copy constructor as it was a requirement of my school to dynamically allocate memory.

here is my code.

    #include <iostream>

using namespace std;

class Complex
{

public:
    double r, i;
    Complex() : r(0), i(0) {}
    Complex(int r, int i)
    {
        this->r = double(r);
        this->i = double(i);
    }
    Complex(double r, int i)
    {
        this->r = r;
        this->i = double(i);
    }
    Complex(int r, double i)
    {
        this->r = double(r);
        this->i = i;
    }
    Complex(double r, double i)
    {
        this->r = r;
        this->i = i;
    }
    Complex(Complex &c)
    {
        cout<<"cpy constructor called"<<endl;
        r = c.r;
        i = c.i;
    }
    void operator()(float r, float i)
    {
        this->r = r;
        this->i = i;
    }
    void operator()(int r, int i)
    {
        this->r = r;
        this->i = i;
    }
    Complex operator+(Complex &c)
    {
        Complex *c1 = new Complex(r + c.r, i + c.i);
        return *c1;
    }
    Complex operator-(Complex &c)
    {
        Complex *c1 = new Complex(r - c.r, i - c.i);
        return *c1;
    }
    Complex operator*(Complex &c)
    {
        Complex *c1 = new Complex(
            r * c.r + (-1) * (i * c.i),
            r * c.i + i * c.r);
        return *c1;
    }
    Complex& operator/(Complex &c)
    {
        float dem;
        Complex num;
        num = (*this) * * (new Complex(c.r,-c.i));
        dem = c.r*c.r + (c.i*c.i);
        Complex *c1 = new Complex(num.r/dem,num.i/dem);
        return *c1;
    }
    operator string()
    {
        string temp, s;
        temp = to_string(r);
        while (true)
        {
            if (temp[temp.length() - 1] == '0')
            {
                temp.pop_back();
            }
            else
                break;
        }
        if (temp[temp.length() - 1] == '.')
            temp.pop_back();
        s += temp;
        if (i >= 0)
        {
            s += '+';
        }
        temp = to_string(i);
        while (true)
        {
            if (temp[temp.length() - 1] == '0')
            {
                temp.pop_back();
            }
            else
                break;
        }
        if (temp[temp.length() - 1] == '.')
            temp.pop_back();
        s += temp;
        s += 'i';
        return s;
    }
    friend ostream &operator<<(ostream &out, Complex c);
};
ostream& operator<<(ostream &out, Complex c)
{
    out << string(c);
    return out;
}


int main()
{
    Complex c(5, 5);
    Complex d(5,5);
    cout << c/d <<endl;
    cout<< c*d <<endl;
}

and this is the error I encounter after this

test.cpp: In function 'int main()':
test.cpp:123:13: error: invalid initialization of non-const reference of type 'Complex&' from an rvalue of type 'Complex'       
     cout<< c*d <<endl;
            ~^~
test.cpp:31:5: note:   initializing argument 1 of 'Complex::Complex(Complex&)'
     Complex(Complex &c)
     ^~~~~~~
test.cpp:111:10: note:   initializing argument 2 of 'std::ostream& operator<<(std::ostream&, Complex)'
 ostream& operator<<(ostream &out, Complex c)

This is the image to the error

Answer 1

The big problem is lack of const on your references and members. The second problem is not using =default. The third problem is too many overloads. Forth, your new use is wrong. operstor() are all wrong. The last problem is your operator strategy. And don't use namespace std;.

struct Complex

Complex is both default public and basically an aggregate. I'd use struct

{
  double r=0
  double i=0;

you can do default initialization in C++ now.

 Complex()=default;

now this ctor is auto-written, because we initialized the members at declaration. DRY is "don't repeat yourself"; when easy, don't repeat member names.

Complex(double r_in, double i_in):r(r_in),i(i_in){}

ctors should construct, not assign, members.

Drop the other 2 arg ctors. Callers can convert.

Complex(Complex const&c)=default;

const here, and let the compiler write a member-wise copy with =default; DRY.

Complex(Complex &&c)=default;
Complex& operator=(Complex const&c)=default;
Complex& operator=(Complex &&c)=default;

also assignment and moves

Now other operators...

Complex& operator+=(Complex const& o)&{

implement a+=b before +.

  r+=o.r;
  i+=o.i;
  return *this;
}

here we repeat member names; it isn't easy to avoid.

Do the same for -= and *=

Next,

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

this makes a non-member +. It makes a copy of its left hand argument by taking it by value, and a const reference to its right hand argument.

We then increment the left hand copy by the right, and return it.

This pattern chans wonderfully in 99% of situations;

Complex x=a+b+c

becomes

Complex x=a;
x+=b;
x+=c;

after accounting for moves and elision.

And you get a+=b; and a+b by writing only one member-wise operation.

Just don't use your operator(), they are nonsense.

Answer 2

The minimal fix isComplex(Complex &c) -> Complex(const Complex &c) But then there will remain leaks due to the inappropriate usage of new there, after some cleanup:

#include <iostream>

using namespace std;

class Complex
{

public:
    double r, i;
    Complex(double r, double i) : r(r), i(i) {}
    Complex operator+(Complex c)
    {
        return Complex(r + c.r, i + c.i);
    }
    Complex operator-(Complex c)
    {
        return Complex(r - c.r, i - c.i);
    }
    Complex operator*(Complex c)
    {
        return Complex(
            r * c.r + (-1) * (i * c.i),
            r * c.i + i * c.r);
    }
    Complex operator/(Complex c)
    {
        auto num = Complex (*this) * Complex(c.r,-c.i);
        auto dem = c.r*c.r + (c.i*c.i);
        return Complex(num.r/dem,num.i/dem);
    }
    operator string()
    {
        string temp, s;
        temp = to_string(r);
        while (!temp.empty() && temp.back() == '0')
            temp.pop_back();
        if (!temp.empty() && temp.back() == '.')
            temp.pop_back();
        s += temp;
        if (i >= 0)
        {
            s += '+';
        }
        temp = to_string(i);
        while (!temp.empty() && temp.back() == '0')
            temp.pop_back();
        if (!temp.empty() && temp.back() == '.')
            temp.pop_back();
        s += temp;
        s += 'i';
        return s;
    }
    friend ostream &operator<<(ostream &out, Complex c);
};
ostream& operator<<(ostream &out, Complex c)
{
    out << string(c);
    return out;
}


int main()
{
    Complex c(5, 5);
    Complex d(5,5);
    cout << c/d <<endl;
    cout<< c*d <<endl;
}

Answer 3

Repeat the pattern of the code below for your other operators. Note the operator takes the parameter by const ref and returns the result by value:

Complex operator+(Complex const &c)
{
   return Complex(r + c.r, i + c.i);
}

The copy constructor should also be changed to take it's parameter by const ref ie

Complex(Complex const &c)
{
  r = c.r;
  i = c.i;
}