Basic complex number class

Question

As part of my C++ training, I wanted to create a basic complex number class for basic complex number calculations.
The class should have the following:

• constructor (non explicit, for implicit conversions)
• << and >> operators to stream with cout and cin
• ==, != operators to compare complex numbers
• +, +=, -, -=, *, *=, /, /= operators for simple arithmetics
• GetR(), GetI(), SetR(), SetI() functions to access the real and imaginary parts of the number

I am going to work in a place that uses an underscore at the end of every function parameter, so I am sorry in advance for any discomfort it may cause while reviewing my code.

This is really a part of my baby steps in C++, so any feedback about style, design and coding will be appreciated. Attached are both the header file and the cpp file:

---

//  complex.h
//  Written by me on Jan 28, 2015

namespace exercises
{

class Complex;

std::ostream& operator<<(std::ostream&, const Complex&);
std::istream& operator>>(std::istream&, Complex&); // assumes (a,bi) format

bool operator==(const Complex, const Complex);
bool operator!=(const Complex, const Complex);

Complex operator+(const Complex, const Complex);
Complex operator-(const Complex, const Complex);
Complex operator*(const Complex, const Complex);
Complex operator/(const Complex, const Complex);

class Complex
{
public:
    Complex(const double r_=0, const double i_=0);// non explicit on purpse

    // using generated ~tor, cctor, c=tor

    Complex& operator+=(const Complex);
    Complex& operator-=(const Complex);
    Complex& operator*=(const Complex);
    Complex& operator/=(const Complex);

    double GetR()const;
    double GetI()const;

    void SetR(const double);
    void SetI(const double);    

private:
    double m_r;
    double m_i;
};

} // namespace exercises

---

//  complex.h
//  Written by me on Jan 28, 2015

#include <iostream>
#include <cassert>
#include "complex.h"

namespace exercises
{

// ---------- global funcs ----------

bool operator==(const Complex a_, const Complex b_)
{
    return (a_.GetR() == b_.GetR() && a_.GetI() == b_.GetI());
}

bool operator!=(const Complex a_, const Complex b_)
{
    return (a_.GetR() != b_.GetR() && a_.GetI() != b_.GetI());
}

Complex operator+(const Complex a_, const Complex b_)
{
    Complex ret(a_.GetR()+b_.GetR(), a_.GetI()+b_.GetI());
    return ret;
}

Complex operator-(const Complex a_, const Complex b_)
{
    Complex ret(a_.GetR()-b_.GetR(), a_.GetI()-b_.GetI());
    return ret;
}

Complex operator*(const Complex a_, const Complex b_)
{
    double a=a_.GetR(), b=a_.GetI();
    double c=b_.GetR(), d=b_.GetI();

    Complex ret(a*c-b*d, b*c+a*d);

    return ret;
}

Complex operator/(const Complex a_, const Complex b_)
{
    double a=a_.GetR(), b=a_.GetI();
    double c=b_.GetR(), d=b_.GetI();
    assert(c || d);

    Complex ret((a*c+b*d)/(c*c+d*d), (b*c-a*d)/(c*c+d*d));

    return ret;
}

std::ostream& operator<<(std::ostream& os_, const Complex& comp_)
{
    return os_ << '(' << comp_.GetR() << ',' << comp_.GetI() << "i)";
}

std::istream& operator>>(std::istream& is_, Complex& comp_) // assumes (a,bi) format
{
    char ch = 0;
    double r = 0;
    double i = 0;

    is_ >> ch >> r >> ch >> i >> ch >> ch;

    comp_.SetR(r);
    comp_.SetI(i);

    return is_;
}

// ---------- interface funcs ----------

Complex::Complex(const double r_, const double i_): m_r(r_), m_i(i_)
{}

Complex& Complex::operator+=(const Complex o_)
{
    m_r += o_.m_r;
    m_i += o_.m_i;

    return *this;
}

Complex& Complex::operator-=(const Complex o_)
{
    m_r -= o_.m_r;
    m_i -= o_.m_i;

    return *this;
}

Complex& Complex::operator*=(const Complex o_)
{
    double a=m_r, b=m_i, c=o_.m_r, d=o_.m_i;
    m_r = a*c - b*d;
    m_i = b*c + a*d;

    return *this;
}

Complex& Complex::operator/=(const Complex o_)
{
    assert(o_.m_r || o_.m_i);
    double a=m_r, b=o_.m_r, c=m_i, d=o_.m_i;
    m_r = (a*c+b*d)/(c*c+d*d);
    m_i = (b*c-a*d)/(c*c+d*d);

    return *this;
}

double Complex::GetR()const
{
    return m_r;
}

double Complex::GetI()const
{
    return m_i;
}

void Complex::SetR(const double r_)
{
    m_r = r_;
}

void Complex::SetI(const double i_)
{
    m_i = i_;
}

} //namespace exercises

Answer 1

• To avoid bugs like one mentioned by @janos, a common recommendation is to express operator!= in terms of operator==:

  bool operator!=(const Complex a_, const Complex b_) {
      return !(a_ == b_);
  }
  

• The same recommendation goes for many other operators, for example operator+ can and should be expressed in terms of operator+=:

  Complex operator+(Complex a_, const Complex b_) {
      return a_ += b_;
  }
  

  This enforces an important invariant (a = b + c must have the same effect as a = b; a += c), and removes the need to call getters and setters.

• Speaking of getters and setters, in your case they serve no purpose other than hiding names. The access to private members is still unrestricted.

• As a mathematician, I would expect Complex to have at least more methods, namely norm and conjugate.

• const qualification of pass-by-value parameters is pointless.

Answer 2

Bug

The != operator returns true if both the real and imaginary parts are different. That's clearly a bug.

Instead of this:

bool operator!=(const Complex a_, const Complex b_)
{
    return (a_.GetR() != b_.GetR() && a_.GetI() != b_.GetI());
}

You probably meant this:

bool operator!=(const Complex a_, const Complex b_)
{
    return (a_.GetR() != b_.GetR() || a_.GetI() != b_.GetI());
}

Coding style, readibility

A common writing style is to put spaces around operators. For example instead of this:

double a=a_.GetR(), b=a_.GetI();
double c=b_.GetR(), d=b_.GetI();

Complex ret(a*c-b*d, b*c+a*d);

Complex ret(a_.GetR()+b_.GetR(), a_.GetI()+b_.GetI());

Write this way:

double a = a_.GetR(), b = a_.GetI();
double c = b_.GetR(), d = b_.GetI();

Complex ret(a * c - b * d, b * c + a * d);

Complex ret(a_.GetR() + b_.GetR(), a_.GetI() + b_.GetI());

The difference may be subtle, but it makes the code easier to read.

Answer 3

Code Review

I see little point in using getters/setters.
The interface for complex numbers is pretty well defined and is not likely to change (as it is already a couple of hundred years old).

I would make all the external functions here friends of the complex class. Even if you get rid of the getters/setters. This documents the fact that these functions are tightly coupled to the implementation of the class.

You are passing all you parameters by value. Personally I would pass them be reference (with the exception of the times when you want to make a copy). It will probably save you a register.

I hate the look of your identifiers with the trailing underscore.

operator<<

I probably would not have put braces around the expression. If I remember my maths books complex numbers looked like this:

5+6i

Personally That's the format I would have used. Also if there is no real part I would not have expect the initial 5 I would also expect it to read normal number (without the imaginary part).

I would expect all the following to work:

5
6i
5+6i

operator>>

There is no validation on the input operator. That's a bit clumsy. I would make sure that the stream is in a good state andd that each of the characters is what is expected and set the error bit if there was a mistake.

std::istream& operator>>(std::istream& is_, Complex& comp_) // assumes (a,bi) format
{
    char   open = 0;
    double r    = 0;
    char   coma = 0;
    double i    = 0;
    char   theI = 0;
    char   close= 0;

    is_ >> open >> r >> comma >> i >> theI >> close;

    if (is && open == '(' && comma == ',' && theI == 'i' && close == ')')
    {
        comp_.SetR(r);
        comp_.SetI(i);
    }
    else
    {
        is_.setstate(std::ios::failbit);
    }

    return is_;
}