Title: Should weak_ptr or shared_ptr be returned from unique-owner-collection?
Tags: c++;shared-ptr;smart-pointers;weak-ptr
Question: There is the only owner of Entity objects - EntityManager.
class EntityManager
public:
st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0weak_ptr<Entity&gt; vs st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0shared_ptr<Entity&gt; getEntityByID(int ID) const;
private:
st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0vector<st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0shared_ptr<Entity&gt;&gt; entities;
When Entity is need to be used in the program, it gets received from EntityManager by using getEntityByID function.
Entity shouldn't be stored in other places in program as shared_ptr, because EntityManager is the only owner, however it can and is stored in many places as weak_ptr.
My question is if, getEntityByID should return weak_ptr or shared_ptr
My argument for using weak_ptr as return value:
It is clear in getEntityByID function signature that weak_ptr should be used after getting Entity.
My arguments for using shared_ptr as return value:
weak_ptr anyway can be converted to shared_ptr, and can be stored then as
shared_ptr in other places. Aditionally, with every use it would have been converted to shared_ptr using lock function.
When shared_ptr would be used, then in every function it could just pass
shared_ptr. There would be no need anymore to use lock function every use of ptr. If it would decide
to store pointer, then it would just copy it to weak_ptr as data member.
Comment: Or vector of unique pointers and return borrowed raw pointers.

Comment: @BaummitAugen because Entity can be deleted from collection

Comment: @Galik yes, there will be many places that reference is stored after it has been deleted. like Entity(monster) can have target locked on the player. Or Player can have target locked on the monster.

Comment: @Galik yes, that's why I store them as weak pointers in other places

Comment: @M.M u right, im wrong with that uniquess

Comment: @Cheersandhth.-Alf "Manager is often a design smell". In reality in the program it is called "Storage" - and its only used to store objects. "Don't introduce needless inefficiencies and checks" - what do you mean exactly?

Comment: @Cheersandhth.-Alf Do you mean it about lock function in weak_ptr?

Comment: Why are you using shared/weak pointers? Is it possible that callers will try to use the pointers after their resource has been deleted?

Comment: Well I would be tempted to return the *weak pointer* as this is what you intend other components to store, only to convert to shared pointers when needed

Comment: Remember if other components store *shared pointer* then entities will never get deleted. *(Java style memory leak)*

Comment: If the `EntityManager` is a unique owner, why not have an `st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0vector` and return references?

Comment: "Manager" is often a design smell. Determine the lifetime guarantees. Accordingly return either a raw pointer or a shared_ptr (in the latter case adjust the preconceived notion of single owner). Don't introduce needless inefficiencies and checks, that's just silly.

Comment: @JohnLock: Yes, lock introduces inefficiency, so if you can avoid it, do. The way locking works is that the shared_ptr CONTROL BLOCK for the object is retained as long as there are weak_ptrs to it, even after the object itself has been logically destroyed. If you use `make_shared` then the control block and the object resides in the same memory block, which thus isn't deallocated until the last weak_ptr to it has disappeared, and if you use ordinary `new` and put the result in a shared_ptr then that allocates a control block, i.e. 2 slow dynamic allocations instead of 1. weak_ptr is expensive.

Comment: @JohnLock if entity may be deleted from collection but should then be owned by a user  , then it is not "unique ownership"


Here is another answer: shared_ptr is for shared ownership.  Any object stored in a shared_ptr csnnot assume it has the unique right to determine object lifetime.
Even if everyone stores weak_ptr, whenever they use it they convert to shared_ptr, and while using it someone could attempt to delete it in the central manager.  This will fail, because a 'temporary' shared_ptr exists.
In a medium complex situation this will occur, whike simple tests will not expose it, leading to later on hard to reason about bugs.
You can use weak/shared to solve this, but only as implementation details: semantics must be enforced on top of the smart pointers.  And those semantics will determine the smart pointers used.
Comment for this answer: yes, I havent looked on it from this side. I thought its "unique" because only `EntityManager` should store it as shared_ptr, but yes. even if function stores it temporary on the stack, it is temporary owner. "Any object stored in a shared_ptr csnnot assume it has the unique right to determine object lifetime" - true. So do you think that I should return shared_ptr from getEntityByID function, in that situation?

Here is another answer: If like you said EntityManager is the only owner of Entities you should store Entities as unique_ptr, not shared_ptr. After changing to vector of unique_ptr, you should return raw pointer from .get() method. Ideally EntityManager could store Entites as objects not pointers and return reference or const reference to stored object.
Comment for this answer: If EntityManager will be the only one storing Entities as shared_ptr and get will return weak_ptr, then after deleting Entity in EntitieManager all weak_ptr will fail on lock(). If you need weak_ptr in your implementation it means that you have bad design.
Comment for this answer: @JohnLock weak_ptr need was designed due to circular dependencies in code between shared_ptrs. If as you said it is predicted situation I would change shared_ptr to unique_ptr and pass raw pointers as observer pointers. There is no difference between raw and weak_ptr in your design.
Comment for this answer: As I have wrote in the comment, Entity can be deleted from EntityManager collection. References to Entity can still exist in the program, like Entity(Player) can have locked target on other Entity(Monster). That's why I use shared pointers and weak pointers.
Comment for this answer: "after deleting Entity in EntitieManager all weak_ptr will fail on lock()" - yes, and its predicted situation in the program. "If you need weak_ptr in your implementation it means that you have bad design" - suppose there is Entity(Human), and Rocket - which stores reference to target it has locked - that Human for example. If as reference(in Rocket) to target, raw pointer or reference would be used, then if Human dies, then Rocket will store reference to invalid object. If shared_ptr would be used, and Human would die, it would prelong life of human.
Comment for this answer: Added(comment too long): If weak_ptr is used, and human dies, then it just die normally, and rocket has just then empty target.
Comment for this answer: You talk here about st75b5:f2c1:f5d7:b954:8dc3:cee9:baf:a8c0observer_ptr? But it only wraps raw pointer. Still, if object gets deleted, then pointers are left refering to undefined objects. With weak_ptr situations when object does not exists anymore, can be handled.
Comment for this answer: And I don't agree that breaking cycles is the only job of weak_ptr. Observing objects without affecting their lifetime is one of common jobs too

{'source': 'StackExchangeDataset', 'subset': 'stackoverflow.com', 'metadata': {'AnswerCount': 2, 'CommentCount': 14, 'ContentLicense': 'CC BY-SA 3.0', 'CreationDate': '2016:00:05/22/16', 'FavoriteCount': 0.0, 'PostTypeId': 1, 'Tags': '<c++><shared-ptr><smart-pointers><weak-ptr>', 'Title': 'Should weak_ptr or shared_ptr be returned from unique-owner-collection?', 'ViewCount': 1295, 'id': 37374236, 'user_AboutMe': None, 'user_AccountId': 7270291.0, 'user_DisplayName': 'John Lock', 'user_Reputation': 573.0, 'user_Views': 289.0, 'user_id': 5543597.0}}