Why am I unable to get iterator to set element with find?

Question

I am trying to find object in set and then call that objects method. Code looks like this:

   void ExpenseManager::addCategory(const string &userName, const string &categName){
      Client temp(userName);
      impl->clientsSet.find(temp)->addNewCategory(categName);
   }

This method is in Expensemanager class. impl here is pointer to inner class, where clientsSet set is defined (I store Client objects in there). addNewCategory() is a method in Client class.

I get an error at impl position saying: "Error 1 error C2662: 'void ExpenseManagerNamespace::Client::addNewCategory(const std::string &)' : cannot convert 'this' pointer from 'const ExpenseManagerNamespace::Client' to 'ExpenseManagerNamespace::Client &'"

Any ideas?

EDIT: Inner class and contructor:

   class ExpenseManager::Implementation{
   private:
      set<Client> clientsSet;
      set<Client>::iterator clientPosSet;
      friend class ExpenseManager;
   };

   // Constructors/destructor-----------------------------------------------
   ExpenseManager::ExpenseManager()
      : impl(new Implementation()){
   }

Meanwhile, check this out: http://stackoverflow.com/questions/8266054/how-can-i-improve-this-design-that-forces-me-to-declare-a-member-function-const — Kristian Duske, Nov 27 '13 at 14:55
And the declaration of `impl`? This is important because its likely a problem due to constness — Sebastian Hoffmann, Nov 27 '13 at 14:56
@Paranaix impl(new Implementation()). Look at the constructor :) — Marius, Nov 27 '13 at 14:57
Thats the initialization, notice that `const Foo* bar = new Foo();` is correct code. — Sebastian Hoffmann, Nov 27 '13 at 14:58
Not directly related to your current problem, but if `find` ever returns `end()` (that is, if the item is not found), you would have UB. — Zac Howland, Nov 27 '13 at 15:02
@Paranaix So I changed my constructor to: `ExpenseManager::ExpenseManager(){ const Implementation* impl = new Implementation(); }` It didn't help. @ZacHowland it's okay for now. It is impossible for it to return `end()` in my scenario :) — Marius, Nov 27 '13 at 15:07
eh? The const is probably the problem. Beside that, you are assigning a local variable, not a member. — Sebastian Hoffmann, Nov 27 '13 at 15:08
@Paranaix I removed const. Still the same issue. All I need to do is just to call Clients method and that is it. Clients object is in set and I need to find it in order to call the method. Any ideas on how to edit the code and find the solution to this problem? — Marius, Nov 27 '13 at 15:12

score 3 · Answer 1 · answered Nov 27 '13 at 15:04

3

Elements of a set are immutable since, in general, changing them will change their sort position within the set. Your choices include, in approximately increasing levels of nastiness:

Use a map, splitting the client data into an immutable identifier and mutable data;
Remove the element and reinsert a modified copy;
Declare the mutable data mutable and the function to modify it const;
Use const_cast to allow modification.

The last two are only an option if you're sure that the modification won't affect its position in the set, otherwise you will break the set.

Also, don't forget to check whether the client was found (i.e. that find didn't return clientsSet.end()) before trying to access it.

answered Nov 27 '13 at 15:04

Mike Seymour

249,747
28
448
644

since when? `find` returns a normal `iterator` not `const_iterator` type (if non const) – Sebastian Hoffmann Nov 27 '13 at 15:05
@Paranaix C++11 added a version of find that returns a `const_iterator`. – Zac Howland Nov 27 '13 at 15:07
but only if the set itself is const, thats normal coding practice – Sebastian Hoffmann Nov 27 '13 at 15:07
1

@Paranaix: Since always. C++11 23.2.4/5 says "For set and multiset the value type is the same as the key type. [...] Keys in an associative container are immutable." There may be a type called `iterator`, but it can't be used for modification. – Mike Seymour Nov 27 '13 at 15:08
Though a regular iterator is returned AFAIK. While you might be right that one will get trouble if changing the state of the object in such a degree that the order becomes corrupted, this should cause a runtime and not a compiletime error. – Sebastian Hoffmann Nov 27 '13 at 15:10
1

@Paranaix: The "regular" `set::iterator` doesn't allow modification, and attempting to do so gives a compile-time error, as one would expect from a violation of const-correctness - and as we see in this question. – Mike Seymour Nov 27 '13 at 15:12

score 0 · Accepted Answer · edited May 23 '17 at 11:49

In C++11, std::set::find has an overload that returns a const_iterator. It appears to be calling that overload instead of the version that returns an iterator, so when you attempt to change the data, you are getting the const error.

The reason for that likely has something to do with more strict enforcement of making values in a set behave like keys (e.g. immutable). A related question can be found here.

Basically, if you want to modify an element, you should remove it from the set, modify it, and re-insert it, unless you can be absolutely sure that your change will not affect the comparison for your set. If you can be sure that the change will not break your set, you can use a const_cast to give you write-access to the element.

Why am I unable to get iterator to set element with find?

2 Answers2