CODEWITHSUNDEEP
c++sortingcomparisonoperator-overloading
The General
The General

Reputation: 1359

< operator returning true when it shouldn't

I'm trying to sort some enums into ascending order using the sort function (sort(cards.begin(), cards.end());) but it doesn't seem to be sorting them correctly. It is returning them in the order:

card3   {rank=SEVEN (7) suit=CLUBS (0) }
card4   {rank=TWO (2) suit=HEARTS (2) }
card5 = {rank=NINE (9) suit=SPADES (3) }

I suspected it was a problem with my comparison operator overloading, which is this:

inline bool operator<(const Card& c1, const Card& c2)
{
    return c1.getRank() < c2.getRank() &&
           c1.getSuit() < c2.getSuit();
}

as far as I can see, only the right hand side of that comparison is true when comparing card3 and card4 and so it should be returning false but when I card3<card4 it returns true!

I'm probably hideously misunderstanding logic here, but this doesn't make any sense to me....

Thanks for your time :)

Upvotes: 2

Views: 141

Answers (2)

jsantander
jsantander

Reputation: 5102

Typically when you compare things with two items you do it like this:

inline bool operator<(const Card& c1, const Card& c2)
{
    if(c1.getSuit() == c2.getSuit()) {
        return c1.getRank() < c2.getRank()
    } else {
        return c1.getSuit() < c2.getSuit();
    }
}

Edit... I didn't think of the actual resulting order... I guess you want to order first by Suit and then by Rank... so I've just reverted the order in my original post.

Upvotes: 3

juanchopanza
juanchopanza

Reputation: 227390

Your comparison operator does not implement a strict weak ordering, which is a requirement for std::sort. A simple way to implement this is with a lexicographical comparison. This is made easy with std::tie:

#include <tuple> // for std::tie

inline bool operator<(const Card& c1, const Card& c2)
{
  return std::tie(s1.getRank(), c1.getSuit()) < 
         std::tie(c2.getRank(), c2.getSuit());
}

You can also roll out your own in the absence of std::tie. The idea is that you have to handle the case where both ranks are the same.

Upvotes: 5

Related Questions