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

Question

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

If you plan to update it, why do you pass it as a `const` parameter? — Karoly Horvath, Nov 11 '15 at 15:08
I don't mean to be smug, but the question title seems to be an oxymoron — Rajesh, Jul 04 '17 at 06:47

score 2 · Accepted Answer · edited May 23 '17 at 11:51

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)

score 2 · Answer 2 · answered Nov 11 '15 at 15:07

2

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

answered Nov 11 '15 at 15:07

marcinj

48,511
9
79
100

score 2 · Answer 3 · answered Nov 11 '15 at 15:13

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;
}

score 1 · Answer 4 · answered Nov 11 '15 at 15:11

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;

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

4 Answers4