tags:

views:

260

answers:

3

Complex operator*(double m, const Complex & c) { return c * m; }

*On the code above, i'm trying to multiply a constant to a complex number. i receive some errors one of them is [ binary 'operator ' has too many parameters]

ostream & operator<<(ostream & os, Complex & c) {os << c.real <<"," << c.imaginary; return os;}

*Can you tell me what i did wrong on this line as well. Thanks*****

        #include <iostream>
        using namespace std;
        class Complex
        {
        private:
            double real;
            double imaginary;
        public:
            Complex();
            Complex(double r, double i = 0);
            Complex operator*(const Complex & c) const;
            Complex operator*(double mult) const; 
           Complex operator*(double m, const Complex & c)
                   { return c * m; }    
          ostream & operator<<(ostream & os, Complex & c) 
        {os << c.real <<"," << c.imaginary; return os;}

        };
        Complex::Complex()
        {
            real = imaginary = 0;
        }

        Complex::Complex(double r, double i )
        {
            real = r;
            imaginary = i;
        }
        Complex Complex::operator*(const Complex & c) const

        {
            Complex mult;
            mult.imaginary = imaginary * c.imaginary;
            mult.real = real * c.real;
            return mult;
        }

        Complex Complex::operator*(double mult) const
        {
            Complex result;
            result.real = real * mult;
            result.imaginary = imaginary * mult;
            return result;
        }
        int main()
        {
            Complex B(5, 40);
            Complex C(6, 15);
            cout << "B, and C:\n";
            cout << B << ": " << C << endl;
            cout << "B * C: " << B*C << endl;
            cout << "10 * B: " << 10*B << endl;
            return 0;
        }
+2  A: 

Don't declare Complex operator*(double m, const Complex & c) and the ostream one as member functions (aka methods): declare them as friend functions instead! (If you even need them to be friends, which you wouldn't if you had the obvious inline accessor methods for the imaginary and real parts -- but anyway the point is that they have to be outside the class!).

Alex Martelli
A: 

For member functions, operator* and operator<< only take a single parameter, as the Complex object is already implicitly given, i.e.

c * r

translates to

c.operator*(r)

If you wish to have the two argument form, then an external friend function is what you are looking for. Although, as Alex points out, if you have re and im acessors set up, then your external operators need not be friends.

rcollyer
+3  A: 

These two operators have problems:

class Complex {
  // ... 
  Complex operator*(double m, const Complex & c)
  {return c * m;}

  ostream & operator<<(ostream & os, Complex & c)
  {os << c.real <<"," << c.imaginary; return os;}
  // ... 
};

Before anything else: That operator<< isn't supposed to change the complex outputs, so that should be const. (Otherwise you cannot output temporary objects, as they can't be bound to non-const references.)

Since they are non-static member functions, they have an implicit this parameter. With that, they have three parameters. However, both are binary operators. Since you cannot make them static (that's just because the rules say so), you have to implement them as free functions. However, as they are implemented, they need access to private members, so you would have to make them friends of your class:

class Complex {
  // ... 
  friend Complex operator*(double m, const Complex & c);
  friend ostream & operator<<(ostream & os, const Complex & c);
  // ... 
};

Complex operator*(double m, const Complex & c)
{return c * m;}

ostream & operator<<(ostream & os, const Complex & c)
{os << c.real <<"," << c.imaginary; return os;}

On a sidenote, it's possible to implement them inline at the point of the friend declaration, which brings you back almost to your original version:

// note the friend
class Complex {
  // ... 
  friend Complex operator*(double m, const Complex & c)
  {return c * m;}

  friend ostream & operator<<(ostream & os, const Complex & c)
  {os << c.real <<"," << c.imaginary; return os;}
  // ... 
};

However, if you implement a concrete mathematical type, your type's users will expect all operations common for such types to work with it. That is, they will, for example, expect c*=r to simply work. So you will need to overload operator*=, too. But that operator does almost the same as operator*, so it would be a good idea to implement one on top of the other. A common idiom is to implement *= (and += etc.) as member functions (since they change their left argument, it's a good idea for them to have access to its private data) and operator* as non-member on top of that. (Usually that's more efficient then the other way around.):

// note the friend
class Complex {
  // ... 
  Complex& operator*=(double rhs)
  {return /* whatever */;}

  friend ostream & operator<<(ostream & os, const Complex & c)
  {os << c.real <<"," << c.imaginary; return os;}
  // ... 
};

inline Complex operator*(Complex lhs, double rhs) // note: lhs passed per copy
{return lhs*=rhs;}

inline Complex operator*(double lhs, const Complex& rhs)
{return rhs*lhs;}

IMO that's the best solution.


I have. however, a few more things to say:

The way you implemented your multiplication is inefficient:

 Complex Complex::operator*(const Complex & c) const
 {
   Complex mult;
   mult.imaginary = imaginary * c.imaginary;
   mult.real = real * c.real;
   return mult;
 }

When you say Complex mult;, you invoke the default constructor of your class, which initializes the real and imaginary parts to 0. The next thing you do is to overwrite that value. Why not do it in one step:

 Complex Complex::operator*(const Complex & c) const
 {
   Complex mult(real * c.real, imaginary * c.imaginary);
   return mult;
 }

or even to more concise

 Complex Complex::operator*(const Complex & c) const
 {
   return Complex(real * c.real, imaginary * c.imaginary);
 }

Sure, it's just two assignments per multiplication. But then - you wouldn't want to have this in some inner loop of your graphic driver.

Also, your constructors are not implemented The Way It Ought To be(TM). For initializing member data, you should use the initializer list:

Complex::Complex()
  : real(), imaginary()
{
}

Complex::Complex(double r, double i)
  : real(r), imaginary(i)
{
}

While it doesn't make any difference for built-in types like double, it doesn't hurt either and it's good to not to get into the habit. With user-defined types (a somewhat unfortunate name, since it is for all non-built-ins, even types like std::string, which isn't defined by users) that have a non-trivial default constructor, it does make a difference: it's far less efficient.

The reason is that, when the execution passes that initial {, C++ guarantees that your data member objects are accessible and usable. For that, they must be constructed, because construction is what turns raw memory into objects. So even if you do not call a constructor explicitly, the run-time system will still call the default constructors. If the next thing you do is overriding the default-constructed values, you're again wasting CPU cycles.

Finally, this constructor Complex(double r, double i = 0) serves as an implicit conversion operator. That is, if, for example, youe meant to call f(real), but forgot to include it, but there is an f(const complex&) in scope, the compiler exercises its right to perform one user-defined conversion and your call f(4.2) becomes f(Complex(4.2)) and the wrong function is silently called. That's very dangerous.

In order to avoid this, you should mark all constructors explicit that could be called with only one argument:

class Complex {
  // ... 
  explicit Complex(double r, double i = 0)
  // ... 
};
sbi
Your post is excellent, but I'd suggest using a single `operator *` outside the class for multiplying two `Complex` values, and let implicit conversion take care of the case where there's a `double` on one side.
coppro
@coppro: This is possible, but less efficient, since it would unnecessarily construct an object.
sbi
And probably even worse than unnecessarily constructing an object, it would unnecessarily do two multiplications (by a zero imaginary part) and addition/subtraction (of the results).
Steve Jessop
"you invoke the default constructor of your class, which initializes the real and imaginary parts to 0". Although I agree with your code change, it's worth pointing out that the compiler can elide the pointless initialisation, and probably will with enough optimisation flags. At least, gcc 3.4.4 does with -O1. Still, why take the chance, just for the sake of writing extra lines of code? Also, using parameterised constructors centralises the task of ensuring you haven't forgotten a field.
Steve Jessop
Update: my test was with a constructor defined in the class definition. With the constructor in the question, -O3 was needed. Inlining is what makes the constructor easy to optimise away.
Steve Jessop
@Steve: Thanks for your analysis! I, too, find the one-liner easier to read and more idiomatic.
sbi