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

• Loki Astari: Note: There is already a std::complex

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_;
}