6

I've been learning c++. I am stuck with this problem.

I have set that contains a custom struct that contains two long int's a & b. I have a custom comparer struct that compares the numbers and returns true if either a or b is different.

typedef long int li;

struct number {
    number(li a1,li b1): a(a1), b(b1) {}
    li a, b;
};

struct compare {
    bool operator() (const number &lhs, const number& rhs) const{
        return lhs.a != rhs.a || lhs.b != rhs.b;
    }
};

int main() {
    set<number, compare> nums;
    nums.insert(number(1, 2));
    nums.insert(number(1, 1));
    nums.insert(number(2, 1));
    nums.insert(number(1, 2));
    for (auto &i : nums) {
        cout << i.a << " " << i.b << endl;
    }
    return 0;
}

The output here is

1 2

2 1

1 1

1 2

It has two entries of 1 2. Any clarification would be appreciated.

Community
  • 1
  • 1
Srikanth R
  • 410
  • 5
  • 17
  • 7
    `compare` doesn't satisfy [the requirements of strict weak ordering](http://en.cppreference.com/w/cpp/concept/Compare), and therefore your program exhibits undefined behavior. Basically, the comparator should resemble a less-than comparison, not a not-equal one. – Igor Tandetnik Dec 02 '16 at 15:25
  • 1
    If you want to use inequality rather than less-than, you could consider `std::unordered_set`, which needs to be able to calculate a hash and compare for equality. – Martin Bonner supports Monica Dec 02 '16 at 15:28
  • Thanks for the comment. Can someone please enlighten why I have to use lesser than one as opposed to inequality? I'm still kinda confused. – Srikanth R Dec 02 '16 at 15:37
  • 2
    @TheAbsurd consider 7 and 9. 7<9 is true, 7>9 is false. For your comparison, they would both be true. You don't know how std::set handles ordering and detecting duplicates behind the scenes, but many sorting algorithms use "<" to do tricks you can't do with "==" when comparing things. So the end result is that the library writer only guarantees this to work if your supplied function meets the requirements specified by the interface. – RyanP Dec 02 '16 at 15:45
  • @RyanP that makes sense. Thanks a lot. – Srikanth R Dec 02 '16 at 15:48
  • @TheAbsurd misunderstood your question, sorry. If you ask why such requirement is there answer is "you cannot sort sequence only using result of equality operator, but can using less than". – Slava Dec 02 '16 at 15:48

2 Answers2

6

Your comparison function should return whether some element is smaller than another, not whether or not they are equal. (More formally, it must define a "Strict weak ordering" on the elements of your set.)

Use something like

struct compare {
    bool operator() (const number &lhs, const number& rhs) const{
        return std::tie(lhs.a, lhs.b) < std::tie(rhs.a, rhs.b);
    }
};

If you don't care about ordering, you may want to define a suitable hash function for your type and use std::unordered_set.

To avoid future problems like this, make sure to read the docs. They clearly explain what your comparison function is supposed to do.

For reference: std::tie as used above constructs tuples of references to its arguments which can then be compared lexicographically with <. This is an easy, generic and fast way to build some ordering for collections of less-than-comparable stuff.

Baum mit Augen
  • 49,044
  • 25
  • 144
  • 182
  • @TheAbsurd link to `std::tie` docs: http://en.cppreference.com/w/cpp/utility/tuple/tie If that seems to weird to you, a simple `if` construct like in the other answer is fine too for simple cases like this. – Baum mit Augen Dec 02 '16 at 15:32
  • Uh.. your comparator must define a strict weak ordering of less-than, not greater-than. Otherwise you're setting up a set to contain items in their reverse order. (Your code is doing this, but the first sentence claims "greater than") – Andre Kostur Dec 02 '16 at 15:53
  • *"Otherwise you're setting up a set to contain items in their reverse order."* That depends on the order you want (and technically my first sentence was correct either way, though maybe somewhat misleading). :) Edited to make it clearer, thanks. @AndreKostur – Baum mit Augen Dec 02 '16 at 15:57
5

Your comparison function needs to meet strict/weak ordering requirements.

(I actually prefer the answer using std::tie, but this may be more illustrative for newcomers)

bool compare(const number& lhs, const number& rhs)
{
   if(lhs.a < rhs.a)
      return true;
   else if(lhs.a > rhs.a)
      return false;
   else
      return lhs.b < rhs.b;
}
Chad
  • 18,706
  • 4
  • 46
  • 63