0

I created a class in C++ with an implementation for operator<=> and operator==. Their logic is different: operator<=> compares based on one field, and operator== compares based on another field. I added a Student to a set, then called find on that set with a Student that wasn't in the set.

#include <iostream>
#include <set>
#include <string>

using namespace std;

class Student {
  private:
    string name;
    int age;

  public:
    Student (string p_name, int p_age) : name {p_name}, age {p_age} {};
    // Students are ordered in respect to their age
    auto operator<=>(const Student& rhs) const {
        return age <=> rhs.age;
    }
    // A student is equal to another if they have the same name
    bool operator==(const Student& rhs) const {
        return name == rhs.name;
    }
};


int main() {
  Student john {"john", 10};
  Student lucas {"lucas", 10};
  set<Student> my_set;
  my_set.insert(john);
  std::cout << std::boolalpha;
  std::cout << "Does my_set contain lucas? " << my_set.contains(lucas) << std::endl;
}

I want my_set.contains(lucas) to return false, instead, it returns true.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
poipoi
  • 124
  • 7
  • Yes, I think you can overload them separately. Also note that the `<=>` operator overloading is only `c++20`, I'm pretty sure. – cs1349459 Oct 27 '22 at 11:20
  • 1
    please show the broken example with the set ([mcve]) – 463035818_is_not_an_ai Oct 27 '22 at 11:31
  • 6
    TLDR : Bad idea! One rule for operator overloading is : dont change their semantic meaning. It will only lead to confusion. If you want to compare different things then create to functions with clear names describing their behavior. In other words overloading for "convenience" (less typing) is only going to hurt in the end. (And probably make your class inusable with std algorithms) – Pepijn Kramer Oct 27 '22 at 11:32
  • 1
    From the C++ core guidelines : [use-an-operator-for-an-operation-with-its-conventional-meaning](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c167-use-an-operator-for-an-operation-with-its-conventional-meaning) – Pepijn Kramer Oct 27 '22 at 11:37
  • What is the 'wanted behaviour'? Do you really want an ordered set (if so what ordering?), or an equality-based unordered one? – bcsb1001 Oct 27 '22 at 11:40
  • 1
    "Find returned an iterator pointing to the node in the set ..." your description is not clear, because neither your `<=>` nor your `==` match the usual notion of equality and for both it is to be expected that `find` can find an element when called with an element not in the set – 463035818_is_not_an_ai Oct 27 '22 at 11:45
  • @463035818_is_not_a_number I want find to compare nodes with operator==, not operator< or other operators – poipoi Oct 27 '22 at 11:51
  • 1
    then you do not want to use `std::set` with the default comparator. `std::set` uses `<` by default. The issue is in your code using the set, we can only help you with this code if you show the code. – 463035818_is_not_an_ai Oct 27 '22 at 11:53
  • @463035818_is_not_a_number Thank you! I included a link with a runnable example in my post – poipoi Oct 27 '22 at 11:54
  • a link does not suffice. Please include the code in the question together with expected and actual output – 463035818_is_not_an_ai Oct 27 '22 at 11:54
  • 3
    C++ frequently does not provide bumper guards. You are allowed to do things that pundits could agree is a bad approach, but are not forbidden by the language. *The Principle of Least Surprise* (POLS) is a convention, and one that the language does not require. I suggest that violating POLS be accompanied by a big comment that explains **what** the odd thing is, and **why** the code is doing the odd thing. – Eljay Oct 27 '22 at 12:40
  • I just wanted to know why this happens, not if it's bad practice or not – poipoi Oct 27 '22 at 12:52
  • 1
    Late addition for late readers: Actually it's even [*encouraged/enforced*](https://stackoverflow.com/questions/61488372/why-default-three-way-operator-spaceship-generates-equality-operator) to have different *implementations* – but they should *semantically* do the same thing (see @PepijnKramer 's comments), i.e. the results of `x < y || y < x` and `x != y`, for instance, should match. Being able to overload separately is intended for being able to provide optimal performance if equality can be implemented more efficiently than the three-way operator. – Aconcagua Oct 27 '22 at 15:20
  • @Aconcagua this could be an answer. So x < y || y < x is used instead of x==y somewhere? (First time using c++) – poipoi Oct 27 '22 at 15:37
  • 1
    `std::set` never uses `==` – 463035818_is_not_an_ai Oct 28 '22 at 07:58
  • 2
    @poipoi If one is smaller (or greater) than the other it cannot be equal, so it *could* be used for inequality. *Could* – if there's an equality operator it's usually more efficient. My point is: These two alternative variants (`< || >` vs. `!=`) should yield the *same result*, no matter which one you finally use! `std::set` can live with just the 'less' relation as for `if(value < currentNode) goLeft(); else if(currentNode < value) goRight(); else equalityThusNodeFound();` – Aconcagua Oct 28 '22 at 16:07

1 Answers1

0

Operators are just function calls; you can have them do anything you want, within the limitations of the language. It is within your rights to make your code as incoherent as you like.

However, once you give such incoherent code to some other code (like, say, the standard library), you are now at the mercy of that other code's expectations. And those expectations tend to be "things do what they say they do".

It is therefore always best to make operators do what they are supposed to do. If for some values a and b such that operator<=>(a, b) != 0, then operator==(a, b) == false too. That will be the expectation of pretty much any code not under your control which intends to use either of these operators.

std::set<T> is an associative, ordered container; this means that it puts the T items in an order and maintains this order automatically as items are added or removed. The ordering is defined by a comparison object given to the template, which defaults to std::less<T> (aka: use operator<). Since operator< is based on operator<=>, that is what is used to define comparisons.

All comparisons. Two values a and b are equal when comp(a, b) == comp(b, a). Since comp here is std::less<T>, it will always use operator< to determine equality. That is, two objects are equal if neither is less than the other.

You cannot make std::set use operator== for this (it only takes one comparison function). Nor would set work if you did, since the internal structure of the type presumes that equivalence is based on the comparison function's ordering. That is, the reason why set::find is O(log(n)) is because all of the objects are put into an order based on the comparison function.

So not only is what you want just bad code, it is code that could never work with any associative container. If you want to write incoherent code, you can, but the consequences of writing incoherent code are that the rest of the world expands and demands coherence.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982