Issue with Overloading Assignment Operator

Question

So the issue I'm running into is in my c++ program, when I try and test the Assignment Operator that I overloaded.

The statement:

cm6 = cm5;

should set cm6 equal to cm5, but the value of cm6 doesn't change. Here's my code:

Proj_11.h

#include <iostream>
#include <string>
#include <iomanip>

using namespace std;

class ComplexNumber
{
    private:
        double _real;
        double _imaginary;
    public:
        ComplexNumber();
        ComplexNumber(double, double);
        ComplexNumber(ComplexNumber&); 
        ComplexNumber operator+ (ComplexNumber);
        ComplexNumber operator- (ComplexNumber);
        ComplexNumber operator* (ComplexNumber);
        ComplexNumber operator/ (ComplexNumber);
        ComplexNumber operator= (ComplexNumber);
        bool operator== (ComplexNumber);
        friend ostream& operator<< (ostream&, ComplexNumber);
};

void Menu();

Proj_11.cpp

#include "Proj_11.h"

//Global Constants
const double NOTHING = 0.0;
const double INVERSE = -1.0;
const int CURRENCY_FORMAT = 2;

void main()
{
   Menu();
}

void Menu( )
{
    // Create complex numbers to do arithmentic with
    ComplexNumber cm1(1, 2);
    ComplexNumber cm2(1, -2);

    // test addition operator
    ComplexNumber cm3 = cm1 + cm2;
    cout << cm3 << endl;

    // test subtraction operator
    ComplexNumber cm4 = cm1 - cm2;
    cout << cm4 << endl;

    // test multiplication operator
    ComplexNumber cm5 = cm1 * cm2;
    cout << cm5 << endl;

    // test division operator
    ComplexNumber cm6 = cm1 / cm2;
    cout << cm6<< endl;

    // test assignment operator
    cm6 = cm5;
    cout << cm6 << endl;

    // test comparison operator
    if (cm1 == cm2) 
        cout << "\nThey are equal.\n";
    else
        cout << "\nThey are not equal.";

    ComplexNumber cm8(1, 2);
    if (cm1 == cm8) 
        cout << "\nThey are equal.\n";
    else
        cout << "\nThey are not equal.";

    system ("PAUSE");
}

ComplexNumber::ComplexNumber()
{
    _real = 0.0;
    _imaginary = 0.0;
}

ComplexNumber::ComplexNumber(double initReal, double initImaginary)
{
    _real = initReal;
    _imaginary = initImaginary;
}

ComplexNumber::ComplexNumber(ComplexNumber& cmplx)
{
    _imaginary = cmplx._imaginary;
    _real = cmplx._real;
}

ComplexNumber ComplexNumber::operator+ (ComplexNumber x)
{
    double newReal = _real + x._real;
    double newImaginary = _imaginary + x._imaginary;
    ComplexNumber temp(newReal, newImaginary);
    return temp;
}

ComplexNumber ComplexNumber::operator- (ComplexNumber x)
{
    double newReal = _real - x._real;
    double newImaginary = _imaginary - x._imaginary;
    ComplexNumber temp(newReal, newImaginary);
    return temp;
}

ComplexNumber ComplexNumber::operator* (ComplexNumber x)
{
    double newReal = 0.0;
    double newImaginary = 0.0;
    //(a+b)*(c+d) = ac+bc+ad+bd
    newReal = newReal + (_real * x._real);
    newImaginary = newImaginary + (_imaginary * x._real);
    newImaginary = newImaginary + (_real * x._imaginary);
    newReal = newReal + (INVERSE * (_imaginary * x._imaginary) );
    ComplexNumber temp(newReal, newImaginary);
    return temp;
}

ComplexNumber ComplexNumber::operator/ (ComplexNumber x)
{
    double newReal = 0.0;
    double newImaginary = 0.0;
    ComplexNumber conjugate(x._real, (INVERSE * x._imaginary));
    ComplexNumber numerator = (*this * conjugate);
    ComplexNumber denominator = (x * conjugate);
    newReal = numerator._real / denominator._real;
    newImaginary = numerator._imaginary / denominator._real;
    ComplexNumber temp(newReal, newImaginary);
    return temp;
}

ComplexNumber ComplexNumber::operator= (ComplexNumber x)
{
    ComplexNumber temp(x._real, x._imaginary);
    return temp;
}

bool ComplexNumber::operator== (ComplexNumber x)
{
    if ( (_real == x._real) && (_imaginary == x._imaginary) )
    {
        return true;
    }
    else
    {
        return false;
    }
}

ostream& operator<< (ostream& out, ComplexNumber x)
{
    out.setf(ios::fixed);
    out.precision(CURRENCY_FORMAT);
    if ( (x._real != NOTHING) && (x._imaginary != NOTHING) )
    {
        if ( (x._real > NOTHING) && (x._imaginary > NOTHING) )
        {
            out << x._real << " + " << x._imaginary << "i";
            return out;
        }
        else if ( (x._real > NOTHING) && (x._imaginary < NOTHING) )
        {
            out << x._real << " - " << (INVERSE * x._imaginary) << "i";
            return out;
        }
        else if ( (x._real < NOTHING) && (x._imaginary > NOTHING) )
        {
            out << x._real << " + " << x._imaginary << "i";
            return out;
        }
        else
        {
            out << x._real << " - " << (INVERSE * x._imaginary) << "i";
            return out;
        }
    }
    else if ( (x._real == NOTHING) && (x._imaginary != NOTHING) )
    {
        out << x._imaginary << "i";
        return out;
    }
    else if ( (x._real != NOTHING) && (x._imaginary == NOTHING) )
    {
        out << x._real;
        return out;
    }
    else
    {
        out << NOTHING;
        return out;
    }
}

`main` must return `int`. And **never** put a using directive in a header. You should also be returning a reference from that operator overload, and actually modifying the object. Speaking of which, the other ones like `operator/` don't modify the object, so they should be marked `const`. — chris, Dec 19 '13 at 00:02
Please get a decent C++ textbook. There are a mind numbing number of serious mistakes all over your code. — Kerrek SB, Dec 19 '13 at 00:05
@user3117263 _' I'll do that in the future.'_ Right now, would be the better choice ... — πάντα ῥεῖ, Dec 19 '13 at 00:09
Speaking of `operator=`, have a look at the [copy swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). — chris, Dec 19 '13 at 00:10
The best way to implement the assignment operator for this class is to simply let the compiler take care of it. Don't declare it, don't define it. Same with the copy constructor. — Benjamin Lindley, Dec 19 '13 at 00:15
@πάντα ῥεῖ Well now is the future from that last comment. — Creoix, Dec 19 '13 at 00:19
@BenjaminLindley I agree, but it's an assignment for class. I have to use the overloaded operator, even if once I get it working, it does the same thing. — Creoix, Dec 19 '13 at 00:21
We'll see if I can even do that though. I'm pretty new to programming, so I'm still getting things down :) — Creoix, Dec 19 '13 at 00:21

score 3 · Accepted Answer · answered Dec 19 '13 at 00:07

3

Your declaration of the assignment operator signature should be

 ComplexNumber& operator= (const ComplexNumber&);

and return a reference to the current instance:

 ComplexNumber& operator= (const ComplexNumber& other)
 {
     // copy members from other ...
     return *this;
 }

... amongst a lot of other errors you'll face with your code.

answered Dec 19 '13 at 00:07

πάντα ῥεῖ

1
13
116
190

Thank you! Looks like I've got a lot more reading up on Overloading Operators, lol. – Creoix Dec 19 '13 at 00:34

score 1 · Answer 2 · answered Dec 19 '13 at 00:15

Copy constructor should take a const& not a &.

operator= should modify this state.

Your stream out is ugly, and you should not mess with precision that way. Set precision outside. It probably has other problems.

unary - is better than *INVERSE in many ways.

Get rid of NOTHING for every reason, including 0.0.

score 1 · Answer 3 · answered Dec 19 '13 at 00:28

What you do is what you get. You change the assigned object nowhere. Instead of this

ComplexNumber ComplexNumber::operator= (ComplexNumber x)
{
    ComplexNumber temp(x._real, x._imaginary);
    return temp;
}

it would be better to write the copy assignment operator the following way

ComplexNumber & ComplexNumber::operator= ( const ComplexNumber &x )
{
    if ( this != &x )
    {
        _real = x._real;
        _imaginary = x._imaginary;
    }

    return ( *this );
}

Issue with Overloading Assignment Operator

3 Answers3