How to write o good Comparison operator ?

[ysf007]

hi,

I'm trying to check if two objects are equal cause I got this error XD

binary '==' : no operator found which takes a left-hand operand of type 'Foo' (or there is no acceptable conversion)

so I wrote this operator==

class Foo{
   private :

    int get_index;
    bool get_found;
    string get_Name;
    int get_matche;
    int get_indexBin;
    int get_Block;
    int get_indexLocation;
    int get_length;




bool operator ==(Foo & o1){

	    if( o1.get_index() == this->get_index() &&
			 o1.get_found() == this->get_found() &&
			 o1.get_Name() == this->get_Name() &&
			 o1.get_matche() == this->get_matche() &&
			 o1.get_indexBin() == this->get_indexBin() &&
			 o1.get_Block() == this->get_Block() &&
			 o1.get_indexLocation() == this->get_indexLocation() &&
			 o1.get_length() == this->get_length())
	        return true;
	    else 
	        return false;
	}

// getter are bellow :)
...
}	

is there any better way to make this equality checking ?

[MikeyBoy]

That's not an == operator. It's a != operator.

EDIT: Note, the OP edited their code after I posted this, to fix the error, in case anyone's wondering what the hell I'm talking about!

To the OP, please don't do that - it makes it really hard for people to understand the replies that were made to what you originally posted.

[ysf007]

Am I mistaken, I want to test if they equal using this

bool operator ==(const Feature & o1, const Feature & o2)

EDIT : I'm sorry @MikeyBoy, I've done a mistake in the code !

[jonnin]

you are mistaken. this is == for class FOO not class feature. Are you trying to do inheritance? This is risky; you can mess it up and only the base classes are compared instead of the whole thing if done wrong, be careful or even pull them out of the class entirely.

do you need to do something that uses a lot of comparisons? Like sorting a million of them?

if so, you are shooting yourself in the foot. Consider wrapping the atomic classes in a struct and using memcmp or something like that (you will have to do the string apart from the atomics, of course).

If you only do a handful, you can leave it as-is.

lose the if-else pair. Just say return (expression). The expression evals to a bool.

comparisons are one place where you can forget how complicated you made it and do a bunch of them somewhere else producing slowness.

[ysf007]

No I'm not using inheritance, I just want to know is there any better way instead of this :

bool operator ==(Foo & o1){

	    if( o1.get_index() == this->get_index() &&
			 o1.get_found() == this->get_found() &&
			 o1.get_Name() == this->get_Name() &&
			 o1.get_matche() == this->get_matche() &&
			 o1.get_indexBin() == this->get_indexBin() &&
			 o1.get_Block() == this->get_Block() &&
			 o1.get_indexLocation() == this->get_indexLocation() &&
			 o1.get_length() == this->get_length())
	        return true;
	    else 
	        return false;
	}

[jonnin]

well, as I said then ... at the risk of making it a bit less readable, you can do a faster comparison with a memcmp or something for the atomics.

I said struct before but this would be even cleaner:
vector<int>vars (8);

int &get_index = vars[0];
int &get_found = vars[1]; //are you ok with changing the type here?
//if not, you can either take the bool outside like the string, or you could check your implementation and ensure that bool is 1 byte and map 1 byte from the int with some pointer magic. If you do that be sure to zero init the full integer so they can be compared safely.
int &get_matche = vars[2];
etc

...
return( o1.vars == this.vars) && (o1.get_Name == this->get_Name); //hopefully vectors use memcmp under the hood? If not, use an array or steal a pointer to the data in the vector and DIY?

[Ganado]

@ysf: Define "better". What about your current method is bad, in your opinion?

If two Foos are equal and only iff found, name, matche, indexbin, block, indexLocation, and length are equal, then that's what you have to compare.

jonnin is assuming you're talking about efficiency. I don't think you are, but just in case: Comparing ints instead memset is not going to grant you any notable advantage, and will ending up costing you portability, maintainability, and extendability. How would a string even turn into an int (unless you're using some restricted set of strings)? If you have a problem with efficiency, properly profile/time/debug your code to see where the actual bottleneck is.

[jonnin]

yes, I was saying efficiency. and I said you have to do the string apart, if you re-read it. I was just trying to combine all those ints together. It may not matter. As I said it depends on how many times he calls the comparisons whether its worth tweaking. I guess just take away a warning: comparisons are an easy place to hide a bottle neck. Ive seen time and time again where a bad compare operator + sort = doh. Complicated comparison operators set off a red flag to me, possibly unwarranted … premature optimizations again!

[JLBorges]

Technically, using memcmp() to compare the values of objects of types other than narrow character types is non-conforming.

C++:

For narrow character types, all bits of the object representation participate in the value representation. ... For unsigned narrow character types, each possible bit pattern of the value representation represents a distinct number. These requirements do not hold for other types.

The C standard notes that:

It is possible for objects x and y with the same effective type T to have the same value when they are accessed as objects of type T, but to have different values in other contexts. In particular, if == is defined for type T,
then x == y does not imply that memcmp(&x, &y, sizeof (T)) == 0.

In short, x == y tests the equivalence of the value representations of x and y
std::memcmp( std::addressof(x), std::addressof(y) ) tests if their object representations are identical.

[ysf007]

thank you guys that was helpful :)

[dhayden]

If you're looking for efficiency then you might also consider which data members are most likely to be different and compare them first.

When you need an == operator, you frequently need the other comparison operators too. When that's the case, I usually write a cmp() method that works similarly to string::compare() or strcmp(): It returns an int <0, 0, or >0 depending on whether the left object is <, ==, or > than the right object. Once you have cmp(), creating the operators is trivial.