C++ set method: function 'setCost' not viable: 'this' argument has type 'const value_type'

Question

I am not able to set value to a private member variable through set method. Getting error

member function 'setCost' not viable: 'this' argument has type 'const value_type' (aka 'const Position'), but function is not marked const

I have below code:

class Position {
public:
    Position();
    Position(int x, int y);
    int getCost() const;
    void setCost (int c);
private:
    int x;
    int y;
    int cost;
    };


void Position::setCost (int c){
    this->cost = c;
}

class Board{
public:
    Board();
    Board(int N);
    void shortestPath32 (Position start, Position end);
private:
    int N;
    char W[32][32];
};

void Board::shortestPath32 (Position start, Position end){

  /* some code here */

    set <Position> validMoves = getValidPositions(parent);
    for(auto child =validMoves.begin(); child!=validMoves.end(); ++child ){
        /*some code here ...*/  
        int c = 5
        (*child).setCost(c);

        }

    }
}

Clearly if I declare setCost as void Position::setCost (int c) const, I am not able to do the assignment operation inside. Also, I looked into this thread for set method but wasn't helpful.

You're trying to mutate the elements of a set. The set doesn't allow that, because it'd screw up the set's internal organization, as well as possibly violating element uniqueness. — user2357112, Jan 29 '17 at 19:23
Why is the cost part of the Position, anyway? It might be better to have a map from positions to costs. — user2357112, Jan 29 '17 at 19:24

rustyx · Answer 1 · 2017-01-29T20:36:03.337

2

That's a limitation of std::set - its iterators always return const references. The rationale is - modifying an element in a set can change its position, so it's not allowed.

To modify an element in a set, the official process is to take it out of the set, modify it, and insert it back in.

Now if you know that modifying certain element properties won't affect its position, as a dirty workaround you can declare those mutable and the setter const:

class Position {
public:
    Position();
    Position(int x, int y);
    int getCost() const;
    void setCost (int c) const { cost = c; }
private:
    int x;
    int y;
    mutable int cost;
};

An even dirtier solution is to cast away const, then you can modify anything (I feel dirty even bringing this up).

P.S. This issue can usually be avoided in the first place by choosing a structure that fits your needs better - for example std::map; you could refactor the code into Position and Cost:

class Position {
    int x;
    int y;
    . . .
};
class Cost {
    int cost;
    . . .
};

std::map<Position,Cost> validMoves;

Then you will be able to modify Cost legally, while Position can remain const:

  for(auto it =validMoves.begin(); it!=validMoves.end(); ++it){
      it->second.setCost(c);
  }

But that's a design choice that may depend on other factors not mentioned in the question...

edited Jan 29 '17 at 20:36

answered Jan 29 '17 at 19:32

rustyx

80,671
25
200
267

Just to make it clear: direct modification of data in `set`, `map` (or any other similar container) may lead to data corruption. – Michał Walenciak Jan 29 '17 at 19:52
1

To be more precise, modifying the part of the *key* that affects the position of an element in a `set` or a `map` **will** lead to data corruption, and modifying any other part **will not** lead to data corruption. The *key* part are the fields used by `operator<` (for trees) or by hashing/equality functions (for hashes). – rustyx Jan 29 '17 at 19:59
"*`begin()` and `end()` always return `const` iterators"* - That wording is a bit sloppy. They do not return `const iterator`, nor `const_iterator`. The `const` overloads do return `const_iterator`, and so do `cbegin` and `cend`, but the non-`const` overloads of `begin` and `end` just return `iterator`. What's true is that for a `std::set`, both the `iterator` and `const_iterator` member types are what the standard (§23.2.4/6) calls "constant iterators". It also says: "It is unspecified whether or not `iterator` and `const_iterator` are the same type", but they "have identical semantics"*. – Christian Hackl Jan 29 '17 at 20:07
1

Thanks, updated. I meant *constant iterators* indeed. – rustyx Jan 29 '17 at 20:17

score 1 · Answer 2 · edited May 23 '17 at 12:01

1

As per the doc, The value of the elements in a set cannot be modified once in the container (the elements are always const), but they can be inserted or removed from the container.

So you need to erase and re-insert in the set.

How to update an existing element of std::set?

edited May 23 '17 at 12:01

Community

1
1

answered Jan 29 '17 at 19:29

Amit Kumar

1,477
1
16
27

score 0 · Answer 3 · answered Jan 29 '17 at 19:46

As others have mentioned you could cast away the const but that wouldn't be the best solution and if you did that you would have to ensure that the cost isn't used in the sort. You could replace set with map and store the cost outside of your class.

You could then do something like the following...

void Board::shortestPath32 (Position start, Position end){
  map<Position, int> validMoves; //getValidPositions(parent);    
  for(auto child=validMoves.begin(); child!=validMoves.end(); ++child ){  
    child->second=1; // NYI - replace 1 with the cost    
  }  
}

C++ set method: function 'setCost' not viable: 'this' argument has type 'const value_type'

3 Answers3