# creating operators...

This topic is 4887 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

Hi there! I'm curious to why the commented line works, and the other dont...


class vector3d
{
public:
double x,y,z;

operator *=(vector3d B)
{
x *= B.x;
y *= B.y;
z *= B.z;
}

operator *=(double B)
{
x *= B;
y *= B;
z *= B;
}

operator *(vector3d B)
{
x *= B.x;
y *= B.y;
z *= B.z;
}

};

void main()
{
//Works:
vector3d NewPos = PosA;
NewPos *= PosB;

//Fails, "cannot convert 'int' to 'vector3d'"
vector3d NewPos = PosA*PosB;

//Works, but result is garbled (x,y,z all equals 1232456 or something similar.... :)
PosA = PosB*PosB;
}



##### Share on other sites
Probably because you forgot the return type and return statement in your operator definititions. They should look more like:
vector3d operator*(vector3d B){    vector3d newVector;    // ... Set newVector's fields to the appropriate values ...    return newVector;}

Also, it looks like your * operator is acting like a *= operator.

##### Share on other sites
I've got operators for both *= and * .... :)

But anyway, you were dead on with the return type thingy, I guess I shouldn't be programming when I'm tired...
Works great now! :)

##### Share on other sites
There are a few problems:
1. No return values. Operators must generally return the results of the operation.
2. Parameters should be const references. References for efficiency. Const so that literals and temporaries work.
3. operator*() is just broken.
Here are improved versions of your operators.
    vector3d & operator *=(vector3d const & B)    {        x *= B.x;        y *= B.y;        z *= B.z;                return *this;    }    vector3d & operator *=(double B)    {        x *= B;        y *= B;        z *= B;                return *this;    }    vector3d operator *(vector3d const & B)    {        vector3d temp = *this;        temp *= B;                return temp;    }

##### Share on other sites
Quote:
 operator *=(vector3d B)

vector3d& operator *=(const vector3d& B)

Less data copied → more efficient code.

##### Share on other sites
Your operator* should return a new vector3d with the multiplied values, not multiply the values of the object it is called on. Currently your code has this behaviour:
vector3d a = {3, 0, 0};vector3d b = {7, 0, 0};vector3d c = a * b;// a = {21, 0, 0}// b = {7, 0, 0}// c = (assumed){21, 0, 0}

which is counterintuitive to say the least. Instead implement the canonical form of the multiplication operator, which is the free function:
vector3d operator*(vector3d lhs, vector3d const & rhs){	return (lhs *= rhs);}

Now multiplication behaves correctly and as a bonus if you change your vector3d class (i.e. to enforce that all vectors are normalised) you only have to update operator*= and operator* comes along for free.

Enigma

##### Share on other sites
Okay, thanks for all the replies!

Current code looks like this:

class vector3d{public:double x,y,z;vector3d& operator +=(const vector3d &B)   {   x+=B.x;   y+=B.y;   z+=B.z;   return *this;   }vector3d& operator +=(const double &B)   {   x+=B;   y+=B;   z+=B;   return *this;   }vector3d& operator -=(const vector3d &B)   {   x-=B.x;   y-=B.y;   z-=B.z;   return *this;   }vector3d& operator +(const vector3d &B)   {   x+=B.x;   y+=B.y;   z+=B.z;   return *this;   }vector3d& operator -(const vector3d &B)   {   x-=B.x;   y-=B.y;   z-=B.z;   return *this;   }vector3d& operator -=(const double &B)   {   x-=B;   y-=B;   z-=B;   return *this;   }vector3d& operator /=(const vector3d &B)   {   x/=B.x;   y/=B.y;   z/=B.z;   return *this;   }vector3d& operator /=(const double &B)   {   x/=B;   y/=B;   z/=B;   return *this;   }vector3d& operator /(const vector3d &B)   {   x/=B.x;   y/=B.y;   z/=B.z;   return *this;   }vector3d& operator /(const double &B)   {   x/=B;   y/=B;   z/=B;   return *this;   }vector3d& operator *=(const vector3d &B)   {   x*=B.x;   y*=B.y;   z*=B.z;   return *this;   }vector3d& operator *=(const double &B)   {   x*=B;   y*=B;   z*=B;   return *this;   }vector3d& operator*(const vector3d &B)   {   x*=B.x;   y*=B.y;   z*=B.z;   return *this;   }vector3d& operator*(const double &B)   {   x*=B;   y*=B;   z*=B;   return *this;   }bool operator ==(const vector3d &B)   {   return (x == B.x && y == B.y && z == B.z);   }bool operator !=(const vector3d &B)   {   return (x == B.x && y == B.y && z == B.z);   }vector3d& operator=(const double B)   {   x = B;   y = B;   z = B;   }};

It seems to work alright now... I cant believe I forgot the "&" sign! :D

##### Share on other sites
The *, /, + and - operators are still messed up. They're modifying this instead of creating a temporary object and returning that (they are acting like *=, /=, += and -=, respectively).

##### Share on other sites
Aaah... thanks for pointing that out!! :D

I was only checking the answers, not the original vectors too... *smacks forehead*

well, be back in a sec...

##### Share on other sites
Quote:
 Original post by RasmadrakOkay, thanks for all the replies!Current code looks like this:*** Source Snippet Removed ***It seems to work alright now... I cant believe I forgot the "&" sign! :D

The binary operators +,-... create a new value, they don't modify the object. You can't return by reference:

vector3d operator*(const double &B){   return vector3d(*this) *= B;}

Though they are better implemented as non-member functions.

vector3d operator*(vector3d vec, const double &B){   return vec *= B; // The copy is made when the argument is passed by value.}

Passing a mere double parameter by const reference may or may not be a good idea. You should benchmark.

• ### What is your GameDev Story?

In 2019 we are celebrating 20 years of GameDev.net! Share your GameDev Story with us.

• 14
• 14
• 45
• 22
• 27
• ### Forum Statistics

• Total Topics
634044
• Total Posts
3015211
×