Re: operator overloading question
none <mikem891@hotmail.com> wrote:
As a style issue. I find it odd that you abbreviate imaginary to img and
yet use COMPLEXTYPE which is usually abbreviated to just T, Ty, or Type.
Also, using two Cs in the name of the class is confusing, just call it
"Complex" not "Ccomplex".
Lastly, at least look at the official interface for the standard complex
class:
http://www.dinkumware.com/manuals/default.aspx?manual=compleat&page=compl
ex.html#complex
template <class COMPLEXTYPE>
class Ccomplex
{
public:
COMPLEXTYPE real;
COMPLEXTYPE img;
// Constructor
Redundant comment, of course the member-function is a constructor.
Ccomplex (void)
Remove "void" above. It is unnecessary.
http://www.parashift.com/c++-faq-lite/newbie.html#faq-29.4
{
real = 0;
img = 0;
}
Get in the habit of using initializer lists instead of assignment in the
body of the constructor.
http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.6
// Constructor (conversion operator)
Redundant comment...
Ccomplex (COMPLEXTYPE val)
{
real = val;
img = 0;
}
What about a two argument constructor? That would make sense...
// Destructor
~Ccomplex (void) { }
The compiler provides a destructor that does exactly what yours does. So
remove yours and let the compiler do its job.
inline Ccomplex<COMPLEXTYPE> operator = (Ccomplex<COMPLEXTYPE>
source);
The compiler provides an op= that does exactly what yours does. So
remove yours and let the compiler do its job.
friend inline Ccomplex<COMPLEXTYPE> operator + \
(Ccomplex<COMPLEXTYPE> dest, Ccomplex<COMPLEXTYPE>
source);
friend inline Ccomplex<COMPLEXTYPE> operator + \
(Ccomplex<COMPLEXTYPE> dest, const COMPLEXTYPE
source);
friend inline Ccomplex<COMPLEXTYPE> operator + \
(const COMPLEXTYPE dest, Ccomplex<COMPLEXTYPE>
source);
The backslash isn't necessary in the above (or anywhere else if this
program for that matter. Don't complicate your life by using unnecessary
constructs.
"friend" isn't necessary. There is nothing private/protected in the
class.
"inline" isn't necessary. The compiler will inline where it can.
The three declarations above aren't necessary.
protected:
Spurious access specifier.
};
typedef Ccomplex<double> COMPLEX;
Reserve all upper-case declarations for macros.
template <class COMPLEXTYPE>
inline Ccomplex<COMPLEXTYPE> \
Ccomplex<COMPLEXTYPE>::operator = (Ccomplex<COMPLEXTYPE>
source)
{
this->real = source.real;
this->img = source.img;
return *this;
}
template <typename COMPLEXTYPE>
inline Ccomplex<COMPLEXTYPE> operator + \
(Ccomplex<COMPLEXTYPE> dest, Ccomplex<COMPLEXTYPE> source)
'dest' and 'source' are misleading names for the arguments.
{
Ccomplex<COMPLEXTYPE> newdest;
newdest.real = dest.real + source.real;
newdest.img = dest.img + source.img;
return newdest;
If you make a two argument constructor, the above can be made much
simpler...
return Complex<T>( left.real + right.real, left.img + right.img );
}
template <typename COMPLEXTYPE>
inline Ccomplex<COMPLEXTYPE> operator + \
(Ccomplex<COMPLEXTYPE> dest,const COMPLEXTYPE source)
{ return dest + (Ccomplex<COMPLEXTYPE>) source; }
Better to call the conversion constructor explicitly.
template <typename COMPLEXTYPE>
inline Ccomplex<COMPLEXTYPE> operator + \
(const COMPLEXTYPE dest, Ccomplex<COMPLEXTYPE> source)
{ return (Ccomplex<COMPLEXTYPE>) dest + source; }
const is unnecessary as used above. Generally, declaring arguments
"const type" is considered bad form. Pass the arguments either by value,
or by const reference.
int main(int argc, char* argv[])
If you don't use the arguments, don't put them in.
{
COMPLEX complex;
COMPLEX complex2;
complex.real = 5;
complex.img = 2;
// WORKS
complex2 = (COMPLEX) 1 + complex;
complex = complex2 + (COMPLEX) 2;
Prefer calling the conversion constructor explicitly instead of using a
cast.
// WORKS but must use double notation to avoid
// template ambiguities between in and double
complex2 = 1.0 + complex;
complex = complex2 + 2.0;
return 0;
"return 0;" from main is unnecessary.
}
So, if you accept all my changes, you end up with:
template <class T>
class Complex
{
public:
T real;
T img;
Complex(): real(0), img(0) { }
Complex(T val): real(val), img(0) { }
Complex(T real, T img): real(real), img(img) { }
};
template <typename T>
Complex<T> operator +(Complex<T> left, Complex<T> right)
{
return Complex<T>(left.real + right.real, left.img + right.img);
}
template <typename T>
Complex<T> operator +(Complex<T> left, T right)
{
return Complex<T>(left.real + right, left.img);
}
template <typename T>
Complex<T> operator +(const T left, Complex<T> right)
{
return Complex<T>(left + right.real, right.img);
}
typedef Complex<double> ComplexDouble;
int main(int argc, char* argv[])
{
ComplexDouble complex;
ComplexDouble complex2;
complex.real = 5;
complex.img = 2;
complex2 = ComplexDouble(1) + complex;
complex = complex2 + ComplexDouble(2);
complex2 = 1.0 + complex;
complex = complex2 + 2.0;
}