0

I have a class named Group:

class Group
{
    int id;
    string addr;
    set<int> members;
    ...
};

I have pointers to multiple groups stored in these containers:

vector<Group*> grpVec
map<int, Group*> grpIdMap 
map<string, Group*> grpAdMap  

I am storing the pointers like this:

//create and populate group object pointer Group *grp
grpVec.push_back(grp)
grpIdMap.insert(std::pair<int,Group*>(grp->id, grp)) 
grpAdMap.insert(std::pair<string,Group*>(grp->addr, grp))

Now, I want to update a group object. If I update only the pointer in the grpIdMap container, will the same object pointer in all other containers get updated?

//Will this update the same pointer object in grpVec and grpAdMap?
grpIdMap.find(1)->second->members.insert(99) 

Is there any issue in this approach?

cppcoder
  • 22,227
  • 6
  • 56
  • 81
  • 1
    Looks like you should be using shared pointers, i.e. `std::shared_ptr` and not raw pointers. – PaulMcKenzie Jul 20 '15 at 06:10
  • All these pointers, what actually stores the objects? – Jonathan Potter Jul 20 '15 at 06:15
  • Here is the issue with your approach: If the `grp` is dynamically allocated, which container is responsible for deallocating the memory? – PaulMcKenzie Jul 20 '15 at 06:21
  • @PaulMcKenzie Like the way I am updating a single container to update everywhere, I would iterate through any single container and deallocate everything once the use is over. – cppcoder Jul 20 '15 at 06:49
  • @cppcoder Your design is flawed. You will deallocate everything in a single container, but then you have pointers to invalid memory in the other containers. What if you forget to remove all those pointers from these containers? To spare all of this work, use `std::shared_ptr`, which quite honestly is what it looks like you should be using. – PaulMcKenzie Jul 20 '15 at 09:26
  • @PaulMcKenzie I do not have C++11 or boost. Is there any solution without using any of them? – cppcoder Jul 20 '15 at 09:49
  • @cppcoder What compiler do you have? – PaulMcKenzie Jul 20 '15 at 14:48
  • gcc 4.8.3 is my compiler – cppcoder Jul 20 '15 at 16:33
  • @cppcoder That compiler has `shared_ptr`. http://stackoverflow.com/questions/8171444/c-stdshared-ptr-usage-and-information – PaulMcKenzie Jul 20 '15 at 21:55

3 Answers3

2

If the pointers all point to the same object, then yes you can use any pointer to change that object.


Lets look at it a little "graphically":

+-------------------+
| pointer in vector | -----\
+-------------------+       \
                             \
+--------------------+        \     +---------------+
| pointer in one map | -------->--- | actual object |
+--------------------+        /     +---------------+
                             /
+----------------------+    /
| pointer in other map | --/
+----------------------+
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 3
    It may be worth highlighting to @cppcoder that if you were to modify the `id` or `addr` members of the `Group` instance, it would _not_ change the indexing in the `grpIdMap` or `grpAdMap` objects. The old index would need to be removed and a new index added. – paddy Jul 20 '15 at 06:21
1

This approach will update the object. If there is any pointers to this object (not copies of it) then they will get updated info.

I recommend you to stop using raw pointers and read how to use smart pointers here: Which kind of pointer do I use when?

And I think you should look this one: http://en.cppreference.com/w/cpp/memory/shared_ptr

P.S. Use encapsulation in your classes. Other objects should not know about how the data is stored inside of you class, just provide interface.

P.P.S. If you want all members to be public, maybe you want to use struct rather than class

Community
  • 1
  • 1
  • 1
    `shared_ptr` is the safe option but does come with a performance hit which might not be desirable in this particular scenario. I like your point about encapsulation here, which is the best advice. In that case, a vector of `unique_ptr` may be more appropriate, and the encapsulating class can then deal with raw pointers in the maps. – paddy Jul 20 '15 at 06:29
  • @paddy, I agree with you, `unique_ptr` must be a default option. Looking for other types of pointers should be based on the time measurements and concrete reasons. – Nikolay Zhulikov Jul 20 '15 at 06:35
  • I did not get how to use `unique_ptr`. Can you show an example for my code? – cppcoder Jul 20 '15 at 09:46
  • I do not have C++11/boost. So I would prefer a solution without them. – cppcoder Jul 20 '15 at 09:49
0

It is changing the object's value that the pointer point to, not pointer value.So as long as all pointers point to the same object, it'll works

user2477
  • 896
  • 2
  • 10
  • 23