Problems implementing operator==

[LyMc]

I have a problem trying to generate a custom == operator for a class I've created, and cannot see what the problem is. The relevant code is in the following snippets:

Color.h:

class Color {
	public:
					Color			();									// constructors
					Color			(COLORREF);
					Color			(int);
					Color			(COLORREF, int);
					~Color			();									// destructor

					Color			(const Color&);						// Copy constructor
		Color&		operator=		(const Color&);						// operators
		bool		operator==		(const Color);
		bool		operator!=		(const Color);
		...

Trim.h:

#include "Color.h"

class Trim {
		...constructors...

		bool			operator==		(const Trim&);
		bool			operator!=		(const Trim&);
		... other member function prototypes...
		
		void*		parent;							// data members
		long		bspace;
		Color		color1, color2,
					color3, color4, color5;
		...

Trim.cpp:

#include "Color.h"
#include "Trim.h"

	... other constructor, destructor implementations ...

bool Trim::operator!= (const Trim& a) { return !(*this == a); }
bool Trim::operator== (const Trim& a)
{
	return a.color1 == color1			<----- error C2678: binary "=="; no operator found which takes a 
		&& a.color2 == color2			<-----	left-hand operand of type 'const Color' 
		&& a.color3 == color3			<-----	(or there is no acceptable conversion) -- repeated on each line
		...
		;
}

I've tried declaring the arg to Color:perator== as "(const Color)" and as "(const Color&)" (as it is defined in Trim.h for the Trim class), with the same results, and the same problem applies where I try to compare a couple of Trims using == or != in other modules. I've seen on various web pages either argument form used, but nowhere is there a statement or explanation I've been able to find that tells which argument form is correct. The operator= works just fine.

The compiler is MSVS .NET 2002, using the SDK only (no STD, .NET or MFC).

 

[uolj]

Your operator== should really be a nonmember function, but if you leave it as a member function you need to make it const.

The proper prototype as a member function is:

bool operator==(const Color&) const;

The error occurred because the method is not const, so the const color in the trim class cannot call the non-const member method.

 

[cpjust]

You should always declare your operator==() functions as non-member non-friend functions. Ex.

class Color
{
   ...
};

bool operator==( const Color& lhs, const Color& rhs )
{
   // Do your comparison here using the public member functions of Color.
}

The reason you got that error is because the left hand side of the == comparison is a const Color& and your operator==() member function isn't a const function.

 

[ArkM]

If you want to compare Color with Color only (and no casting to Color from another types), you may declare ==() et al operators as Color members free. Don't forget to add const - that's all.
Non-member/non-friend approach may result in less efficient code for complex classes.

 

[cpjust]

The benefit of declaring the operator classes as non-member/non-friend classes is that if you want to support comparing types that can be implicitly converted to a Color class, you can do that with both of these ways:

COLORREF cr = 0x01020304;
Color c( cr );

// 1. Compare cr and c.
if ( cr == c ) { ... }

// 2. Compare c and cr.
if ( c == cr ) { ... }

If operator==() was a member function you would only be able to use the 2nd method, but not the first.

 

[cpjust]

Sorry, I just re-read my last post and thought it might be a little confusing for some, so let me elaborate. To compare using both method 1 and 2 you can still compare both ways even if you don't have an implicit conversion available, but you would need two operator==() functions:

bool  operator==( const SomethingElse& lhs, const Color& rhs ) { ... }
bool  operator==( const Color& lhs, const SomethingElse& rhs ) { ... }

As for the performance difference... If your compiler has a good optimizer, the performance difference should be almost 0. It could also reduce class dependencies.
See Item 44 in "C++ Coding Standards" by Herb Sutter & Andrei Alexandrscu.