operator problems

I've made a vector class which contain the x, y, and z coordinates. The thing is i wanted to compare 2 vectors. This is my vector class

Code:

---

class vect3
{
public:
        double x, y, z;
        vect3()
        {
                x=0; y=0; z=0;
        }


        vect3(double xc, double yc, double zc)
        {
                x = xc;
                y = yc;
                z = zc;
        }

        vect3& operator=(vect3 &v)
        {
                x = v.x;
                y = v.y;
                z = v.z;
                return *this;
        }

---

- So i've been able to give the vector's x, y, z values using operator. But then when i put this compare operator in:

Code:

---

        //compare vector
        vect3& operator ==(vect3 v)
        {
                if(( x == v.x) &&
                  ( y == v.y) &&
                  ( z == v.z))
                  return v;
        }

        vect3& operator <=(vect3 &v)
        {
                if(( x <= v.x)&&
                  ( y <= v.y)&&
                  ( z <= v.z))
                  return v;
        }

        vect3& operator >=(vect3 &v)
        {
                if((x >= v.x) &&
                  (y >= v.y) &&
                  (z >= v.z))
                  return v;
        }

---

Run the operator...

Code:

---

vect3 v1, v2;

if(v1 == v2)
    // statement is true;

---

and get this error

Code:

---

error C2451: conditional expression of type 'class vect3' is illegal
  No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

---

So can anyone help me and point out what the problem is. Thanks

> //compare vector
> vect3& operator ==(vect3 v)

Should this be:

Code:

---

        //compare vector
        bool operator ==(vect3 v)

---

As equality is a boolean operation.

And if you don't want to copy your vector time and again, it should look like this:

Code:

---

bool operator == ( const vect3& v) const

---

And for your comparison functions:

Code:

---

        vect3& operator >=(vect3 &v)
        {
                if((x >= v.x) &&
                  (y >= v.y) &&
                  (z >= v.z))
                  return v;
        }

---

What if the if's condition is false? You won't be returning anything of value.

And shouldn't it be a bool besides?

Maybe you want something like this:

Code:

---

        bool operator >=(vect3 &v)
        {
                return (x >= v.x && y >= v.y && z >= v.z);
        }

---

Thanks guys. I was fixing my code and thought i could do something like this. I think it's not that as good as yours but still look cool:

in the vect3d class i have some functions that compare the vector:

Code:

---

bool vect3::compare(vect3 v1, vect3 v2)
{
        if(        (v1.x == v2.x) &&
                (v1.y == v2.y) &&
                (v1.z == v2.z))
                return true;
        else
                return false;
}



bool vect3::smaller(vect3 v1, vect3 v2)
{
        if(        (v1.x <= v2.x) &&
                (v1.y <= v2.y) &&
                (v1.z <= v2.z))
                return true;
        else
                return false;
}

bool vect3::bigger(vect3 v1, vect3 v2)
{
        if(        (v1.x >= v2.x) &&
                (v1.y >= v2.y) &&
                (v1.z >= v2.z) )
                return true;
        else
                return false;
}

---

then the vector comparation will be run by those function:

Code:

---

        bool operator ==(vect3& v){        return compare(*this, v);}
        bool operator <=(vect3& v){ return smaller(*this, v);}
        bool operator >=(vect3& v){ return bigger(*this, v); }

---

I have to try your codes now. Which one do you think is better?

a couple of opinions/observations about your code..

first.. by adding the extra function calls you are just making the code bigger slower and harder to read, and harder to debug..

the ideal way to overload the comparison operators is to do exactly like was mentioned in the previous posts by nvoigt and dwks...

Code:

---

bool operator >= (const vect& v) const
{
            return (x >= v.x && y >= v.y && z >= v.z);
}

---

you did the same comparisons only putting them inside an if..your way is more expensive but you are on the right track by putting everying in one logical statement..

and please correct me if im wrong, i havent done much 3d work for a while.. but i dont remember any use for comparisons on full vectors except for equality.. but a greater than* or less than* is somewhat undefined.. how can you say a vector is greater or smaller.. you can measure the magnitude, but even that should not be done with an operator..
i mean its no big deal if you want to write these.. just be careful that you dont go and make it more confusing for yourself later on.. you end up shooting yourself in the foot by overloading too many operators you think you might need.. or atleast i do...

i got a quote from somewhere that "tedium is preferred over amiguity".. cant find the source though.. kind of applies here..

Your idea of what is less than or equal is not consistent. What if v1 is (0, 5, 10) and v2 is (6, 5, 4)? Then none of the operators will return true. You should fix the code in the operator so that if v1 < v2 is true, then v2 >= v1 is true.

Also, you should have operator< and operator>. They are commonly used in sorting. I would then implement operator<= as the opposite of operator>.

Your function parameters should all always be const vect3 &. This avoids copying the data everywhere and doesn't allow the functions to accidentally change the variables passed in.

As far as having separate functions called compare, bigger and smaller, you can include those if you like, but they don't serve any real purpose with the code you've shown. What you should do is make the operators non-member functions.

So your function prototypes should be:

Code:

---

bool operator<(const vect3& v1, const vect3& v2); // For you to fix
bool operator>(const vect3& v1, const vect3& v2); // For you to fix
bool operator==(const vect3& v1, const vect3& v2); // For you to fix
bool operator<=(const vect3& v1, const vect3& v2) { return v2 > v1; }
bool operator>=(const vect3& v1, const vect3& v2) { return v2 < v1; }

---

Hint: One common way to implement ordering when there are multiple properties in the class is to order on the first variable first, and only if it is equal order on the next variable, and so on.

Well, the <= and >= operator weren't the one i would use in the program. Some how i just got carry on when i did the ==operator, and said to myself "hey, let's make the other comparations and test them for fun." Thank you anyway by pointing out that those comparations won't work accep for ==operator.

Quote:

---

Your function parameters should all always be const vect3 &. This avoids copying the data everywhere and doesn't allow the functions to accidentally change the variables passed in.

---

Hey Daved, can you explain a little bit more about this, i really didn't get that.

const to make sure nothing is changed on accident and & so that needless copies are not created when "calling" the operator. Objects that are passed by value do not have their constructors called but the deconstructor will be called when the pass by value copy goes out of scope. This will lead to the deconstructor being called too many times (a ton too many times if you return an object).

>> Objects that are passed by value do not have their constructors called but the deconstructor will be called when the pass by value copy goes out of scope.
This isn't entirely correct.

The problem with passing by value (vect3 v1) is that a copy is made. This means that the copy constructor for vect3 is called and the x, y, and z values are copied into the new v1 object. Your code doesn't define the copy constructor, so one is generated automatically by the compiler. The variable v1 s then used and when the function ends it goes out of scope and the destructor is called (which does nothing in this case).

When you pass by reference (vect3& v1), a reference to the original variable is passed to the function. This means that the three double properties are not copied at all, and no constructor or destructor is called. In this case, since vect3 only has a few doubles, it is unlikely to make much of a difference in the efficiency of your program. However, it doesn't really hurt to pass by reference here, so you might as well. The larger the amount of data in the class, the more important it is to pass by reference instead of by value.

The const part is just for const correctness. It ensures that your code doesn't accidentally change the data in the object. Since it is a reference to another variable, you don't want to accidentally change it. If you do try to change a const object, the compiler will produce an error, which is how it protects you from making that mistake. That makes the final version (const vect3& v1).

If all that seems a bit too much to handle, or you're not ready to learn pass by reference yet, you can make your functions pass by value and your code will work fine in this case. A simple class with a few doubles can be treated like a regular type and just passed by value everywhere. In your original code, you mixed and matched, so you should just pick one and stick with it.

I think i may stick with pass by reference because there are some of my other functions use "pass by reference", not by const reference. It may work just fine but i have to be more careful.