CODEWITHSUNDEEP
c++
yihang hwang
yihang hwang

Reputation: 55

Update map which is read-only object using c++

void value_multi(const map<string, double>& myMap, string target,double alva) {

    double res = 0;
    map<string, double>::const_iterator iter = myMap.begin();
    for(; iter!=myMap.end(); ++iter) {
        string key = iter->first;
        vector<string> strs;
        boost::split(strs,key, boost::is_any_of("|"));

        if(strs[0] == target) { //if target == string value
            //res += iter->second;
            double tmp=iter->second*alva; // set tmp
            iter->second=tmp;  //update value
        }
    }

    //return res;
}

I want to update a value using method like code ,but it shows some message after compile

[Error] assignment of member 'std::pair<const std::basic_string<char>, double>::second' in read-only object

Is that any easy and quick way to change my code that will easy update the iter->second value

Upvotes: 1

Views: 2414

Answers (4)

agold
agold

Reputation: 6276

You should use the "normal" iterator instead of a const_iterator, since the later is read only:

map<string, double>::iterator iter = myMap.begin();

See: what is the difference between const_iterator and iterator?

Note that you also will have to change the parameter type const map<string, double>& myMap of your function to a non const type: 

void value_multi(map<string, double>& myMap, string target,double alva)

Upvotes: 2

Edwin Rodr&#237;guez
Edwin Rodr&#237;guez

Reputation: 1257

Use const cast

const_cast<double&>(iter->second) = tmp;

It changed the constness of a variable. Because it's not a good practice to modify a const object, I recommend to change the const modifiers in the map argument like this.

void value_multi(map<string, double>& myMap, string target,double alva) {

    double res = 0;
    map<string, double>::iterator iter = myMap.begin();
    for(; iter!=myMap.end(); ++iter) {
        string key = iter->first;
        vector<string> strs;
        boost::split(strs,key, boost::is_any_of("|"));

        if(strs[0] == target) { //if target == string value
            //res += iter->second;
            double tmp=iter->second*alva; // set tmp
            iter->second=tmp;  //update value
        }
    }

    //return res;
}

Upvotes: 2

Jerry Coffin
Jerry Coffin

Reputation: 490148

You've passed the map by const reference--that's a promise that you won't modify the map. In this case, however, your whole intent is to modify the map.

The cure is simple: pass the map by non-const reference (which would allow a non-const iterator, but see below: the code you have using the iterator should also be eliminated).

The rest of your code is unnecessarily inefficient. Right now, you're walking through every element of the map. At each iteration, you split the target string, and check whether the current map element is equal to the first part of the target string.

I'd write the code more like:

std::string key = target.substr(0, target.find("|"));

auto pos = myMap.find(key);
if (pos != myMap.end())
    pos->second *= alva;

Upvotes: 1

marcinj
marcinj

Reputation: 49986

Your iterator is const:

map<string, double>::const_iterator iter

so you cannot assign / change its contents. Make it non const to do it:

map<string, double>::iterator iter

but then you will also have to make myMap non const

Upvotes: 2

Related Questions