operator problems

This is a discussion on operator problems within the C++ Programming forums, part of the General Programming Boards category; I've made a vector class which contain the x, y, and z coordinates. The thing is i wanted to compare ...

  1. #1
    Computer guy
    Join Date
    Sep 2005
    Location
    I'm lost!!!
    Posts
    200

    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
    Hello, testing testing. Everthing is running perfectly...for now

  2. #2
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    > //compare vector
    > vect3& operator ==(vect3 v)

    Should this be:
    Code:
    	//compare vector
    	bool operator ==(vect3 v)
    As equality is a boolean operation.

  3. #3
    the hat of redundancy hat nvoigt's Avatar
    Join Date
    Aug 2001
    Location
    Hannover, Germany
    Posts
    3,139
    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
    hth
    -nv

    She was so Blonde, she spent 20 minutes looking at the orange juice can because it said "Concentrate."

    When in doubt, read the FAQ.
    Then ask a smart question.

  4. #4
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,046
    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);
    	}
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  5. #5
    Computer guy
    Join Date
    Sep 2005
    Location
    I'm lost!!!
    Posts
    200
    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?
    Hello, testing testing. Everthing is running perfectly...for now

  6. #6
    xhi
    xhi is offline
    !anExpert
    Join Date
    Mar 2005
    Location
    pa
    Posts
    155
    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..

  7. #7
    Registered User
    Join Date
    Jan 2005
    Posts
    7,319
    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.

  8. #8
    Computer guy
    Join Date
    Sep 2005
    Location
    I'm lost!!!
    Posts
    200
    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.

    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.
    Hello, testing testing. Everthing is running perfectly...for now

  9. #9
    Registered User
    Join Date
    Mar 2002
    Posts
    203
    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).

  10. #10
    Registered User
    Join Date
    Jan 2005
    Posts
    7,319
    >> 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.

  11. #11
    Computer guy
    Join Date
    Sep 2005
    Location
    I'm lost!!!
    Posts
    200
    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.
    Hello, testing testing. Everthing is running perfectly...for now

Popular pages Recent additions subscribe to a feed

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21