using object as key of a Map- Error

[ravss2]

I am trying to use object of one class as a key of a map that is used in other class.
But getting the following error, which I could not understand.

error: Passing 'const Team' as 'this' argument discards qualifiers [-fpermissive]


this is the line causing error.

std::cout<<m.first.getnameOfTeam()<<std::endl;

team.h
=======

#ifndef __TEAM__
#define __TEAM__

#include <string>

class Team{
private:
    std::string nameOfTeam;
    std::string nameOfCaptain;
public:
    std::string getnameOfTeam();
    std::string getnameOfCaptain();
    Team();
    ~Team();
    Team(std::string,std::string);
    bool operator <(const Team& t) const;
    friend std::ostream & operator << (std::ostream &out, const Team &k);
    
};
#endif 

team.cpp
=======

#include <iostream>
#include <string>
#include "team.h"
#include "wcup.h"

bool Team :: operator <(const Team& t) const{
        return (this->nameOfTeam > t.nameOfTeam);
}

std::string Team :: getnameOfTeam(){
    return this->nameOfTeam;
}

std::string Team :: getnameOfCaptain(){
    return this->nameOfCaptain;
}

Team::Team(){
    
}

Team::~Team(){
    
}

Team::Team(std::string team,std::string captain) :nameOfTeam(team),nameOfCaptain(captain)
{
    
}

std::ostream &operator << (std::ostream &out,const Team &k)
{
    std::cout <<k.nameOfTeam<<" "<<k.nameOfCaptain<<std::endl;
}
int main()
{
    Team Hurry ("Hurricanes","Nersi Kent");
    Team Murray ("Murrycanes","Mersi Ment");
    WorldCup wcup;
    std::cout<<Hurry.getnameOfTeam()<<" "<<Hurry.getnameOfCaptain()<<std::endl;
    std::cout<<Murray.getnameOfTeam()<<" "<<Murray.getnameOfCaptain()<<std::endl;

    wcup.TeamDescription(Hurry,"Hurrains are the 4th team");
    wcup.TeamDescription(Murray,"Murrays are the 10th team");
    wcup.Display();
    
    return 0;
}

wcup.h

#ifndef __WCUP__
#define __WCUP__
#include<iostream>

#include "team.h"

class Team;
class WorldCup{
private:
    std::string desc;
public:
     void TeamDescription(Team team,std::string msg);
     void Display();
     void GetTeamDetails();
};

#endif 

wcup.cpp

#include "wcup.h"
#include<map>

std::map<Team,std::string> Hurricanes;

void WorldCup::TeamDescription(Team team,std::string msg) {
    Hurricanes.insert(std::make_pair(team,msg));
}

void WorldCup::Display() {
    for(auto &m:Hurricanes)
        std::cout<<"{"<<m.first<<" "<<m.second<<"}"<<std::endl;
}

void WorldCup::GetTeamDetails() {
    for(auto &m:Hurricanes) {
                std::cout<<m.first.getnameOfTeam()<<std::endl;
    }
}

[dutch]

getNameOfTeam and getNameOfCaptain should be const methods.

Also, your operator<< overload should be like this:

std::ostream& operator<<(std::ostream& out, const Team& k)
{
    return out << k.nameOfTeam << ' ' << k.nameOfCaptain << '\n';
}

[ravss2]

ok , I modified get functions to const, error resolved.

But , I am getting same error for one of my set functions, that looks like this

m.first.setnameOfTeam(team);


by the way const methods operate only on const members right?
but none of my members I declared const, so why the requirement of const in this case?

[dutch]

const methods operate only on const members right?

No, that's not right. A const method simply promises not to modify any data members.

As for your other problem, that method is not in the code you've shown. Post the current code.

[ravss2]

Current Code:

I still don't get why we need to make member functions as const, ofcourse its good if we do, but why I am getting error if I don't.

getting error at both these functions definitions in team.cpp

setnameOfTeam

setnameOfCaptain

Team.cpp
===========

#include <iostream>
#include <string>
#include "team.h"
#include "wcup.h"

bool Team :: operator <(const Team& t) const{
        return (this->nameOfTeam > t.nameOfTeam);
}

std::string Team :: getnameOfTeam() const {
    return this->nameOfTeam;
}

std::string Team :: getnameOfCaptain() const {
    return this->nameOfCaptain;
}

void Team :: setnameOfTeam(const std::string team) const {
     this->nameOfTeam = team;
}

void Team :: setnameOfCaptain(const std::string captain) const {
     this->nameOfCaptain = captain;
}
Team::Team(){
    
}

Team::~Team(){
    
}

Team::Team(std::string team,std::string captain) :nameOfTeam(team),nameOfCaptain(captain)
{
    
}

std::ostream &operator << (std::ostream &out,const Team &k)
{
    std::cout <<k.nameOfTeam<<" "<<k.nameOfCaptain<<std::endl;
}
int main()
{
    Team Hurry ("Hurricanes","Nersi Kent");
    Team Murray ("Murrycanes","Mersi Ment");
    WorldCup wcup;
    std::cout<<Hurry.getnameOfTeam()<<" "<<Hurry.getnameOfCaptain()<<std::endl;
    std::cout<<Murray.getnameOfTeam()<<" "<<Murray.getnameOfCaptain()<<std::endl;

    wcup.TeamDescription(Hurry,"Hurrains are the 4th team");
    wcup.TeamDescription(Murray,"Murrays are the 10th team");
    wcup.Display();
    wcup.GetTeamDetails();
    return 0;
}

team.h
=======

#ifndef __TEAM__
#define __TEAM__

#include <string>

class Team{
private:
    std::string nameOfTeam;
    std::string nameOfCaptain;
public:
    std::string getnameOfTeam() const;
    std::string getnameOfCaptain() const;
    
    void setnameOfTeam(const std::string) const;
    void setnameOfCaptain(const std::string) const;
    
    Team();
    ~Team();
    Team(std::string,std::string);
    bool operator <(const Team& t) const;
    friend std::ostream & operator << (std::ostream &out, const Team &k);
    
};
#endif

wcup.cpp

#include "wcup.h"
#include<map>

std::map<Team,std::string> Hurricanes;

void WorldCup::TeamDescription(Team team,std::string msg){
    Hurricanes.insert(std::make_pair(team,msg));
}

void WorldCup::Display() {
    for(auto &m:Hurricanes)
        std::cout<<"{"<<m.first<<" "<<m.second<<"}"<<std::endl;
}

void WorldCup::GetTeamDetails() {
    std::cout<< "Team Details"<<std::endl;
    for(auto &m:Hurricanes) {
                //std::cout<<m.first.getnameOfTeam()<<std::endl;
                std::cout<<m.first<<m.second<<std::endl;
    }
}

void WorldCup::SetTeamDetails() {
    std::cout<< "Team Details After "<<std::endl;
    for(auto &m:Hurricanes) {
                //std::cout<<m.first.getnameOfTeam()<<std::endl;
                if(m.first.getnameOfTeam() == "Hurry")
                    m.first.setnameOfTeam("Jarry");
    }
}

#ifndef __WCUP__
#define __WCUP__
#include<iostream>

#include "team.h"

class Team;
class WorldCup{
private:
    std::string desc;
public:
     void TeamDescription(Team team,std::string msg);
     void Display();
     void GetTeamDetails();
     void SetTeamDetails();
};

#endif

[jlb]

I still don't get why we need to make member functions as const,

Using a const qualifier on a member function insures that the function will not modify the state of the class.

If you plan on your functions changing the state of the class you can't use the const qualifier. For example:

void Team :: setnameOfTeam(const std::string team) const {
     this->nameOfTeam = team;
}

Here you want to be able to change the nameOfTeam variable so the function can't be const qualified.

By the way the function parameter does not have to be const qualified, especially when you are passing the parameter by value, as you are in the above function. However it is usually considered a good practice to pass non-trivial parameters by reference/const reference.

You are, IMO, complicating things by always using the this-> parameter in your member functions.

So this function should really be more like:

void Team::setnameOfTeam(const std::string& /* notice the pass by const reference parameter */ team) /* note the lack of the const qualifier */ {
     nameOfTeam = team;
}

[ravss2]

Ok, changed the setnameOfTeam & setcaptainOfTeam as per your suggestion, now I get the error for the line in wcup.cpp inside SetTeamDetails

passing 'const Team' as 'this' argument discards qualifiers [-fpermissive]

 
 m.first.setnameOfTeam("Jarry");

[dutch]

You can't modify the key (first element) of a map pair since it is const. If you really wanted to do that you would need to remove the element from the map, change the key, and re-insert it. However it's probably not a good idea to do that while looping through the map. I suppose you could insert it into another map and then set the original map to the new one after the loop. Something like:

void WorldCup::SetTeamDetails() {
    decltype(Hurricanes) m;
    for (std::pair<Team, std::string> p: Hurricanes)
        if (p.first.getnameOfTeam() == "Hurry") {
            p.first.setnameOfTeam("Jarry");
            m.insert(p);
        }
    Hurricanes = m;
}

And Hurricanes should probably be a member of WorldCup.

What exactly are you trying to accomplish with this program overall? What is it supposed to do? What you have so far doesn't make much sense to me.

[ravss2]

This recreation of my original program just to resolve my errors.

Actually, I am not into C++ Coding, trying to convert C solution to C++.

This solution is not very big , it just cannot be open in public forum.

If anybody can review the code I can PM the code, please suggest me better approaches.

Thanks,
ravss

[ne555]

    for (std::pair<Team, std::string> p: Hurricanes)
        if (p.first.getnameOfTeam() == "Hurry") {
            p.first.setnameOfTeam("Jarry");
            //m.insert(p);
        }
	m.insert(p);
Hurricanes = m;

also, maps have fast search
auto team = Hurricanes.find( Team("Hurry", "") );

[ravss2]

ne555, Thanks for correcting...
I have actually figured that out already but dint post it, whatever dutch wrote, will remove all other member from the map except the changed one...

But in any case, what we are trying to do is just copy & paste the contents from map, with our modification, but isn't there a better way just change what is required.

For example, let say my Team class has a member matches_won & I want to update that whenever they win a match, so in that case I want to update only that particular team and not other teams.

dutch , hope I made some sense now !!

may be there is a better way to do same thing, but I am just learning...

[MikeyBoy]

I still don't get why we need to make member functions as const

Since I don't think this has been addressed clearly:

If you have a const object, the only methods you can call on that object are the ones that have been defined as const.

This makes complete sense. If you're specifying that an object cannot be modified, then it makes sense to disallow calling methods that would modify it. So, you make the ones that won't modify it const.

[ne555]

> For example, let say my Team class has a member matches_won & I want to
> update that whenever they win a match, so in that case I want to update only
> that particular team and not other teams.
map/set gives you fast searching O(lg n) based on a `less than' operation
which you've defined it like this

bool Team :: operator <(const Team& t) const{
        return (this->nameOfTeam > t.nameOfTeam);
}

that is, you sorted them based on `nameOfTeam'
doing auto it = Hurricanes.find( Team("Hurry", "this string is not relevant and may have any value, even empty") ); you'll get an iterator to the Hurry team stored in your Hurricanes collection
(it->first is the Team)

if you modify the object, then you may screw up the internals of the map (the order may change) so you get a const_iterator which doesn't allow you to modify the object.
you have some alternatives:
- remove/insert:

auto value = *it; //make a copy
Hurricanes.erase(it); //remove the element
value.first.update(); //update whatever you want
Hurricanes.insert(value); //"reinsert" it on the collection 

- cast away:

auto &team = const_cast<Team&>(it->first);
team.update(); //don't modify `nameOfTeam' or you'll screw up the collection 

- change your collection type:
set/map gives you fast insert/erase/search of elements O(lg n)
a sorted vector gives you fast search O(lg n) but slow insert/erase O(n)
¿do you expect to be removing/adding a lot? ¿do you add everything once and then operate? ¿is n quite big (say, in the thousands)?
perhaps you may use vector instead

- change your key:
map< string, Team > where the key is the name of the team.
so then you'll say Hurricanes["Hurry"].update();



> let say my Team class has a member matches_won
> std::map<Team,std::string> Hurricanes;
use meaningful names for your variables

[ravss2]

Thanks ne555 for detailed explanation.

Actually my solution requires embedding the name of team in a message as below

std::string message = {teamname, secretmessage};

but I also need an object to modify the members based on secret message in few of the functions

so I had to change my map to

map<Team,std:string> //object,message

now I have Team object so that can change any of its members based on message.(ofcourse I am not changing the deciding element used for sorting)

if I use map<std:string,Team> , which does not make much sense because message is the key

I hope it is clear, the problem is that, I am trying to re-create a new solution out of my solution ,so it will be difficult for you guys to advice me in right direction

But I really appreciate all the help I got so far atleast some concepts are clear now.

if that's not too much...
I once again request, any of you guys please look at my original code
may be It could be entirely done in a different way such as Inheritance etc...

[ravss2]

I have opened a new post for the relevant solution I am working on here:

http://www.cplusplus.com/forum/general/266966/

please provide your suggestions there, ne555,dutch, jbl,JLBorges