Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ operator < overloading

I have a problem with overloading of the < operator. I have this class:

WordEntry.h:

class WordEntry
{
public:
    WordEntry(string word);
    ~WordEntry();

    bool operator<(const WordEntry otherWordEntry);

    string getWord();

private:
    string _word;
};

WordEntry.cpp(I removed constructor & destructor):

string WordEntry::getWord()
{
   return _word;
}


bool WordEntry::operator<(WordEntry otherWordEntry)
{
   return  lexicographical_compare(_word.begin(),_word.end(),otherWordEntry.getWord().begin(),otherWordEntry.getWord().end());
}

Everything is fine when I'm using it in main.cpp like that:

    WordEntry w1("Der");
    WordEntry w2("das");

    if (w1.operator<(w2)) {
       cout << "w1 > w2";
    }
    else 
    {
       cout << "w2 > w1";
    }

But when I call sort() on a vector with WordEntry objects, I'll get the error message

Invalid operands to binary expression ('const WordEntry' and 'const WordEntry')

and it points to stl_algo.h.

Does anyone knows what's going on here?

like image 729
LeonS Avatar asked Dec 20 '22 23:12

LeonS


2 Answers

Right now the argument to < is const but the member is not. This means a < comparison between 2 const WordEntry& objects will fail because it can't bind to <. You need to make the member and the argument both const

bool operator<(const WordEntry& otherWordEntry) const;

bool WordEntry::operator<(const WordEntry& otherWordEntry) const {
  ...
}

Note: As pointed out in the comments you should also pass WordEntry by reference

like image 130
JaredPar Avatar answered Dec 24 '22 02:12

JaredPar


string WordEntry::getWord()
bool WordEntry::operator<(WordEntry otherWordEntry)
{
   return  lexicographical_compare(_word.begin(),
                                   _word.end(),
                                   otherWordEntry.getWord().begin(),
                                   otherWordEntry.getWord().end());
}

The getWord member function creates a copy of the internal member attribute and returns the copy. Two consecutive calls to getWord will return two different std::string instances with the same contents, but they are different objects none the less. The lexicographical_compare function requires that the first and second arguments are iterators into the same container, and similarly the third and fourth arguments. In your case you are passing iterators into different containers (string), which will be compared inside the function and will yield undefined behavior.

The simplest solution is have getWord return a const reference to the internal std::string, in that way, the iterators will both refer to the internal object in the right hand side object.

As others have also mentioned, you should pass the WordEntry by const reference, and the operator< should be const, to improve the code. But the issue in your implementation is the mixture of iterators from different container.

like image 26
David Rodríguez - dribeas Avatar answered Dec 24 '22 01:12

David Rodríguez - dribeas