# Compare variables and return the relation of the first ones that are not equal in C++ [closed]

I have a C++ function that is called frequently within a loop. It compares the variables v_1, v_2 and v_3 with type double of two objects a and b of a class MyClass in the following way:

// a, b are objects of a class MyClass
// v_1, v_2 and v_3 are class variables of type double

bool compare(MyClass const *a, MyClass const *b) const {

if(a->v_1 != b->v_1) {

return(a->v_1 > b->v_1);   // Usual case
}

else if(a->v_2 != b->v_2) {

return (a->v_2 < b->v_2);  // Happens quite some times
}

else {

return (a->v_3 < b->v_3);  // Unusual case
}
}


I suspect that there is a better/easier way to implement this functionality. Do you have an idea on how to improve both style and, if possible, also efficiency of this function?

• Is it intended that the first comparison uses > while the other 2 use <?
– nwp
Nov 23 '17 at 15:48
• Can you explain at a high level what compare does? It doesn't seem to be simply comparing MyClasss but do something more complicated instead.
– nwp
Nov 23 '17 at 16:26
• @nwp Regarding your first comment, yes that's intended. Nov 23 '17 at 16:50
• @nwp Regarding your second comment, the function is actually a comparison that is used for picking objects of Myclass from a priority queue based on the comparisons shown there. Nov 23 '17 at 16:53
• There really isn't enough context to properly review this - for instance, you've omitted the definition of the (vaguely-named) MyClass. Depending on the types of its members, you might be able to write a clearer one-liner such as return std::tie(b->v1, a->v2, a->v3) < std::tie(a->v1, b->v2, b->v3);. Nov 23 '17 at 17:11

# Naming

MyClass is a poor name for a type - type names should convey what they are used for.

# Encapsulation

The compare() appears to be a member function (as it has a const decoration). If it's a member of MyClass, then it should either be static instead, or it should use the this argument it implicitly receives. And it could be named operator< instead, to increase its utility:

bool MyClass::operator<(const MyClass& other) const


If it's not a member function, then the const is a syntax error. Not only that, but it implies that the members of MyClass are public, which seems wrong for a "class" (as opposed to a struct).

# Implementation

Comparing pairs of values in turn until a pair is unequal is a common pattern, and the usual means to make it short and clear is to make use of the built-in less-than operator of std::tuple, something like this:

// Note that v1 sorts in descending order, so is swapped in the tuples
return std::make_tuple(other.v1,       v2,       v3)
< std::make_tuple(      v1, other.v2, other.v3);


If you don't want to copy the members (perhaps they are of class type), you could use std::tie() instead of std::make_tuple(), to compare through references instead.

• Thank you for your suggestions! The original code has more useful variable names, of course. Regarding encapsulation and implementation, I will try the things you suggested and post again what I've decided to use. As you might notice, I'm a beginner at C++, so thanks for pointing it out the std::tie function! Nov 24 '17 at 9:32