0

I want to overload the left-hand operator (<) for class Node. Note that, the elements won't be the class objects it will be a pointer to them. Please see the set<Node*> defined in the main class.

The one I have written right now doesn't work. I have also tried the friend function, declaring overloading outside the class as a non member function, but it doesn't work either. Doesn't work mean elements are randomly ordered in the set, and the comparator is not called. Whereas, it should be ordered as per the defination of my comparator.

#include <iostream>
#include <set>
using namespace std;

class Node {
public:
    int x, y;
    Node *prev, *next;

    Node(int x, int y) {
        this->x = x; this->y = y;
        this->prev = this->next = nullptr;
    }

    bool operator<(const Node& node) const {
        return this->x < node.x;
    }
};

int main() {
    set<Node*> S;

    S.insert(new Node(2, 4));
    S.insert(new Node(3, 2));
    S.insert(new Node(1, 4));
    S.insert(new Node(5, 1));
    S.insert(new Node(4, 3));

    for (auto itr : S)
        cout << itr-> x << endl;

    return 0;
}
  • Your sample code seems to work https://godbolt.org/z/vKT8WP35b – Etienne Laurin Jan 18 '23 at 05:13
  • 1
    Make it a non-member function, but you almost certainly *don't* want to overload it for the left operand being a pointer. That's almost certainly a mistake. – Jerry Coffin Jan 18 '23 at 05:14
  • Overloaded operators need to have at least one of their operands an instance of a class type. So it is not possible, for example, to overload an `operator<()` that takes two pointers. Your example, however does not overload for pointers since the argument is a *reference*, not a pointer. It also works "as is", as long as the caller does a comparison of the form `some_node < some_other_node`. – Peter Jan 18 '23 at 05:22
  • *"the left-hand operator"* -- the what? Do you mean the left **operand** for the `<` operator? – JaMiT Jan 18 '23 at 05:23
  • *"Please see the set S defined in the class."* -- is this a roundabout way of asking how you can define a [set of pointers to objects with custom comparator](https://stackoverflow.com/questions/67620937/) that will have your desired [order of elements in set of pointers](https://stackoverflow.com/questions/13234106/)? If so, please try to be more direct with your wording and examples, instead of of obscuring what you mean with unnecessary details. *Focus on your goal (the set) instead of your intended approach (overloading something). Failure to do this could land you in an XY problem.* – JaMiT Jan 18 '23 at 05:30
  • @EtienneLaurin I have updated the code on the question itself, it doesn't work. – Milind Prajapat Jan 18 '23 at 05:32
  • @JerryCoffin Actually those parameters may be pointer but I am going to compare them on the basis of attributes, in this case x. I tried making it non-member function as well. It doesn't work either. – Milind Prajapat Jan 18 '23 at 05:34
  • 1
    Please don't say "doesn't work" without giving a precise description of both what is expected and what actually happens. Those facts are important, more than your (perhaps flawed!) interpretation. – Ulrich Eckhardt Jan 18 '23 at 05:59
  • Note : `#include using namespace std;` those are bad practices and should not be used. (and indicates you are not learning C++ from a good quality source) – Pepijn Kramer Jan 18 '23 at 06:03
  • @MilindPrajapat: Yes, that's exactly what's almost always a problem. If you want to take both parameters as pointers, then the compiler simply won't allow it (an overload requires at least one parameter of user-defined type). Otherwise, you're comparing a pointer to an object, which simply doesn't make much sense unless the object in question acts like a pointer (e.g., like `shared_ptr` and `unique_ptr` do). – Jerry Coffin Jan 18 '23 at 06:58
  • @JaMiT I am sorry for incomplete clarity on the question. I have updated it. And the link your shared is an exact solution to my question. – Milind Prajapat Jan 18 '23 at 07:33
  • @JerryCoffin See this [link](https://stackoverflow.com/questions/67620937/set-of-pointers-to-objects-with-custom-comparator). It works now. – Milind Prajapat Jan 18 '23 at 07:34

1 Answers1

0

The problem is, that per default the std::set uses std::less for comparison. Please read here.

And std::less is able to compare Node*. Your comparison function will never be called. Instead. Everything will be sorted according to a (for your random) pointer value. This is the value, that is returned by new.

What you need is a Functor that compares your values in the following form:

    bool operator ()(const Node* n1, const Node* n2) const {
        return (n1->x == n2->x) ? n1->y < n2->y : n1->x < n2->x;
    }

You can use a separate Functor, a Lambda, a free function, or, you can add this function to your Node and, with that, make it a Functor. Then you add this as the 2nd parameter to the std::setdefinition.

So, you have really several ways. Let me use the last option as an example.

Please see:

#include <iostream>
#include <set>
using namespace std;

class Node {
public:
    int x{}, y{};
    Node* prev{}, * next{};

    Node() {};
    Node(int x, int y) {
        this->x = x; this->y = y;
        this->prev = this->next = nullptr;
    }
    
    bool operator ()(const Node* n1, const Node* n2) const {
        return (n1->x == n2->x) ? n1->y < n2->y : n1->x < n2->x;
    }
};

int main() {
    set<Node*, Node> S;

    S.insert(new Node(2, 4));
    S.insert(new Node(3, 2));
    S.insert(new Node(1, 4));
    S.insert(new Node(5, 1));
    S.insert(new Node(4, 3));

    for (auto itr : S)
        cout << itr->x << endl;

    return 0;
}
A M
  • 14,694
  • 5
  • 19
  • 44