CODEWITHSUNDEEP
c++classc++11operator-overloadingstdset
Cpt. Pineapple
Cpt. Pineapple

Reputation: 149

Error: passing x as 'this' argument of x discards qualifiers

I'm writing a HeartbeatManager for my application (in visual studio). The data of 1 heartbeat is stored in a Heart object. The Heart objects are stored in an std::set. For this reason, I am implementing the operator=, operator< and operator> overloads.

When defining these functions you can only use const members. I tried to do this but still get an error saying I'm not:

passing const HeartbeatManager::Heart' as 'this' argument of 'bool HeartbeatManager::Heart::operator<(const HeartbeatManager::Heart&)' 
discards qualifiers [-fpermissive]

Here is the code. I don't see where I'm using a non-constant:

class HeartbeatManager
{
public:
    class Heart
    {
    public:
        Heart(const IPAddress _ip, const uint16_t _port, int _lifetime = 5000)
            : ip(_ip), port(_port), lifetime(_lifetime) {}

        const IPAddress ip;
        const uint16_t port;
        int lifetime;
        /**
        * Ages the Heart, returns whether it survived (lifetime after aging > 0)
        */
        bool age(int ms) 
        {
            this->lifetime -= ms;
            return 0 < this->lifetime;
        }

        // overloaded operators so heart struct can be sorted and used in map
        bool operator=(const Heart& o) {
            return ip == o.ip && port == o.port;
        }

        bool operator<(const Heart& o) {
            return (uint32_t)ip < (uint32_t)o.ip || (ip == o.ip && port < o.port);
        }

        bool operator>(const Heart& o) {
            return (uint32_t)ip > (uint32_t)o.ip || (ip == o.ip && port > o.port);
        }
    };
    void heartbeat(IPAddress ip, uint16_t port, int sec = 5000);
};

I'm very new to C++. So if this is a bad practice for making an object fit the set requirements feel free to let me know.

Upvotes: 2

Views: 758

Answers (3)

JaMiT
JaMiT

Reputation: 16873

The Heart objects are stored in an std::set. For this reason I am implementing the =, < and > operators.

It's fine to define as many operators as you like (more operators do give your class more flexibility). However, the use of std::set justifies only operator<; a default set does not make use of "greater than" or "equal to" (which presumably is what you thought a single equal sign would mean).

When defining these functions you can only use const members. I tried to do this but still get an error saying I'm not:

Take a moment to think about what you are doing. You want to be able to compare two objects using syntax like A < B. As you stated, these should be considered constants by operator<. Emphasis on "these". There are two objects. There should be two const qualifications. Let's take a look at the operator declaration mentioned in the error message:

bool HeartbeatManager::Heart::operator<(const HeartbeatManager::Heart&)

How many times does "const" appear in that declaration? Only once! You need another "const". Where should it go? The error message comes to the rescue again: it complains about the this argument. Whether or not *this is considered to be constant is determined by the presence or absence of the const keyword after the parameter list:

bool HeartbeatManager::Heart::operator<(const HeartbeatManager::Heart&) const

Upvotes: 2

JeJo
JeJo

Reputation: 32847

You are taking a const-qualified lvalue reference to the Heart. The means the function is not mutating the instance passed. Hence they all should be const qualified.

bool operator==(const Heart& o) const
//          ^^^--> typo         ^^^^^
{
    return ip == o.ip && port == o.port;
}
bool operator<(const Heart& o) const
//                             ^^^^^
{
    return (uint32_t)ip < (uint32_t)o.ip || (ip == o.ip && port < o.port);
}
bool operator>(const Heart& o)  const
//                              ^^^^^
{
    return (uint32_t)ip > (uint32_t)o.ip || (ip == o.ip && port > o.port);
}

That being said, your operator< and operator> could be simplified using lexicographical comparison provided by std::tie

bool operator< (const Heart& o) const
{
    return std::tie(ip, port) < std::tie(o.ip, o.port);
}
bool operator> (const Heart& o)  const
{
    return std::tie(o.ip, o.port) < std::tie(ip, port);
}

Upvotes: 1

Sid S
Sid S

Reputation: 6125

You want to allow your compare operators to be called on a const object, so replace

bool operator < (const Heart &o) {

with

bool operator < (const Heart &o) const {

and so on.

Also note that you have implemented operator = () rather than operator == ()

Upvotes: 1

Related Questions