Ah, you got got me wrong there.
The idea of my AnyPtr is just to avoid overloads if one wants to not have a semantic meaning. Just as in "do stuff with X". For example blurring a given Image.
If one should pass a std::shared_ptr, the function should take a std::shared_ptr.
If the function should take a weak pointer, it should take a weak pointer.
The "dirty hack" that Urho is doing with it's T* when T is a child of RefCounted is pretty misleading as a naked pointer has no semantic meaning but the RefCounted naked pointer actually has a semantic meaning. Ideally there should be a type for this slightly different smart pointer.
The AnyPtr should be only used as a replacement for T* (with no semantic meaning) so that also a SharedPtr<T> (and maybe others as well) can be passed without requiring .Get().
To the player with the item example:
You are using an Item* but want to manage that resource. There's nothing hinting towards any kind of resource management as it is just a naked pointer. It would be clear if it would be a SharedPtr<Item> and the Item would be passed as a SharedPtr<Item>. We are in the age of smart pointers and having a naked pointer but wanting any kind of smart behavior from it can be considered a bug.
Again, it re-introduces the issue from my original post. As far as semantics go, what does Terrain::SetHeitmap(AnyPtr<Image>) mean semantically? Does Terrain own that resource afterwards? Or does Terrain hold a weak reference to it? Or does Terrain rely on the parent object's lifetime and simply hold a raw pointer?
In the ideal case: It doesn't do anything with that Image afterwards. It may have modified it or copied it but it is safe to delete that Image directly after SetHeightmap.
I just looked at the code and the SetHeightMapInternal sets a SharedPtr with that given image. So it is using the Image shared but that was not hinted in any way as it only got an Image* originally.This hidden smart pointer stuff is really dirty.
So it kinda boils down to:
- wanting shared ownership without costs at every argument pass
- ideally: wanting to indicate that the thing the pointer is passed to will use it shared (like the SetHeightmap) or not care at all after leaving the function
What's wrong with just calling
- Code: Select all
where SetHeigtMap() accepts a raw pointer Image*? It's clear that we're passing a raw pointer, and it's clear that since Image inherits RefCounted, it is a shared resource, meaning that Terrain could potentially gain ownership of the image passed. There is no confusion in this call.
Two problems: Currently .Get() is not required and removing the implicit cast in SharedPtr to T* would break code. Secondly requiring .Get() everywhere adds a lot of code bloat. Both could be avoided by for example my approach with a converter class.
Also having to distinguish between naked pointers in general and naked pointers with the base RefCounted is pretty misleading. As already said having a zero cost type (like no ref count changes when copied and similar) would be better as it would be also fast (the speed reason you mentioned) but less misleading and add type safety.
I'm currently not seeing a perfect way to solve the issues with functions like SetHeightmap that ideally should take a shared pointer but without the cost.
Seems to be a common issue:http://stackoverflow.com/questions/8385 ... -referencehttp://stackoverflow.com/questions/3275 ... shared-ptrhttp://stackoverflow.com/questions/1082 ... -argumentshttps://herbsutter.com/2013/06/05/gotw- ... arameters/
Though in the case of SetHeightmap the cost for passing a shared pointer by value can be neglected as that function isn't called that often. But there may be similar cases that are actually performance critical.
Besides the neglectable cost there's nothing speaking against Terrain::SetHeighmap(SharedPtr<Image>) am I right?
Hm one option would be Terrain::SetHeighmap(SharedPtr<Image>&). Zero cost at first but also "shareable" due to not being const. Still kinda dirty.