491
votes
#include <iostream>
#include <set>

using namespace std;

class StudentT {

public:
    int id;
    string name;
public:
    StudentT(int _id, string _name) : id(_id), name(_name) {
    }
    int getId() {
        return id;
    }
    string getName() {
        return name;
    }
};

inline bool operator< (StudentT s1, StudentT s2) {
    return  s1.getId() < s2.getId();
}

int main() {

    set<StudentT> st;
    StudentT s1(0, "Tom");
    StudentT s2(1, "Tim");
    st.insert(s1);
    st.insert(s2);
    set<StudentT> :: iterator itr;
    for (itr = st.begin(); itr != st.end(); itr++) {
        cout << itr->getId() << " " << itr->getName() << endl;
    }
    return 0;
}

In line:

cout << itr->getId() << " " << itr->getName() << endl;

It give an error that:

../main.cpp:35: error: passing 'const StudentT' as 'this' argument of 'int StudentT::getId()' discards qualifiers

../main.cpp:35: error: passing 'const StudentT' as 'this' argument of 'std::string StudentT::getName()' discards qualifiers

What's wrong with this code? Thank you!

4
Where is line 35 in your code snippet?In silico
I wish GCC would improve this error message, e.g. "discards qualifiers" -> "breaks const correctness"jfritz42
@jfritz42: Would be confusing for the case it discards volatilePlasmaHH
@PlasmaHH the error message would be split into "breaks const correctness" and "breaks volatile correctness". Now, not many people will think about something being volatile correctCaleth

4 Answers

558
votes

The objects in the std::set are stored as const StudentT. So when you try to call getId() with the const object the compiler detects a problem, mainly you're calling a non-const member function on const object which is not allowed because non-const member functions make NO PROMISE not to modify the object; so the compiler is going to make a safe assumption that getId() might attempt to modify the object but at the same time, it also notices that the object is const; so any attempt to modify the const object should be an error. Hence compiler generates an error message.

The solution is simple: make the functions const as:

int getId() const {
    return id;
}
string getName() const {
    return name;
}

This is necessary because now you can call getId() and getName() on const objects as:

void f(const StudentT & s)
{
     cout << s.getId();   //now okay, but error with your versions
     cout << s.getName(); //now okay, but error with your versions
}

As a sidenote, you should implement operator< as :

inline bool operator< (const StudentT & s1, const StudentT & s2)
{
    return  s1.getId() < s2.getId();
}

Note parameters are now const reference.

92
votes

Member functions that do not modify the class instance should be declared as const:

int getId() const {
    return id;
}
string getName() const {
    return name;
}

Anytime you see "discards qualifiers", it's talking about const or volatile.

6
votes

Actually the C++ standard (i.e. C++ 0x draft) says (tnx to @Xeo & @Ben Voigt for pointing that out to me):

23.2.4 Associative containers
5 For set and multiset the value type is the same as the key type. For map and multimap it is equal to pair. Keys in an associative container are immutable.
6 iterator of an associative container is of the bidirectional iterator category. For associative containers where the value type is the same as the key type, both iterator and const_iterator are constant iterators. It is unspecified whether or not iterator and const_iterator are the same type.

So VC++ 2008 Dinkumware implementation is faulty.


Old answer:

You got that error because in certain implementations of the std lib the set::iterator is the same as set::const_iterator.

For example libstdc++ (shipped with g++) has it (see here for the entire source code):

typedef typename _Rep_type::const_iterator            iterator;
typedef typename _Rep_type::const_iterator            const_iterator;

And in SGI's docs it states:

iterator       Container  Iterator used to iterate through a set.
const_iterator Container  Const iterator used to iterate through a set. (Iterator and const_iterator are the same type.)

On the other hand VC++ 2008 Express compiles your code without complaining that you're calling non const methods on set::iterators.

2
votes

Let's me give a more detail example. As to the below struct:

struct Count{
    uint32_t c;

    Count(uint32_t i=0):c(i){}

    uint32_t getCount(){
        return c;
    }

    uint32_t add(const Count& count){
        uint32_t total = c + count.getCount();
        return total;
    }
};

enter image description here

As you see the above, the IDE(CLion), will give tips Non-const function 'getCount' is called on the const object. In the method add count is declared as const object, but the method getCount is not const method, so count.getCount() may change the members in count.

Compile error as below(core message in my compiler):

error: passing 'const xy_stl::Count' as 'this' argument discards qualifiers [-fpermissive]

To solve the above problem, you can:

  1. change the method uint32_t getCount(){...} to uint32_t getCount() const {...}. So count.getCount() won't change the members in count.

or

  1. change uint32_t add(const Count& count){...} to uint32_t add(Count& count){...}. So count don't care about changing members in it.

As to you problem, objects in the std::set are stored as const StudentT, but the method getId and getName are not const, so you give the above error.

You can also see this question Meaning of 'const' last in a function declaration of a class? for more detail.