std::list< boost::shared_ptr<SceneNode> > _nodes; // List of all scene nodes.
StaticMeshNode* SceneManager::addStaticMeshNode(const char* filename) {
boost::shared_ptr<StaticMeshNode> node(new StaticMeshNode());
node.get()->initialize(filename, _resourceManager, _renderer, this);
_nodes.push_back(node);
return node.get();
}
Question about using boost::shared_ptr in an std::list or vector
Author
I'm writing a scene manager which has an std::list of all of the nodes. Here is some code:
My code compiles and seems to work fine, but I am just a little suspicious that I might be doing something a bit off. I've been reading the boost documentation but I just can't seem to apply it to how it is used in a list or a vector.
Any comments/help would be helpful.
Thanks
Nope, that'll work fine, and will do just what you expect. One thing, though... what's up with having an empty constructor and an "initialize" member? When you do that sort of thing, you make Bjarne Stroustrup cry.
Author
I was actually noticing the same thing while I was implementing the shared_ptrs. I think it was because each scene node needs different parameters to initialize. I realize that isn't necessary now but when I wrote this (a couple years ago) I think that's what I had in mind.
A couple of additional comments:
std::list< boost::shared_ptr<SceneNode> > _nodes; // List of all scene nodes.StaticMeshNode* SceneManager::addStaticMeshNode(const char* filename){ boost::shared_ptr<StaticMeshNode> node(new StaticMeshNode()); // Most smart pointers (and this includes the Boost smart pointers) // are implemented so that they mimic the semantics of 'raw' pointers. // As such, you can use the * and -> operators directly; you don't // need to use the get() function here: node/*.get()*/->initialize(filename, _resourceManager, _renderer, this); _nodes.push_back(node); // Note that if you follow Sneftel's suggestion (which you should), // the above becomes: /* _nodes.push_back(boost::shared_ptr<StaticMeshNode>( filename, _resourceManager, _renderer, this)); */ // Note that you can clean up the code a bit by using a typedef. // Finally, I'm not sure what you're doing here, but generally speaking, // when you invoke the 'get' function, you defeat the purpose of using // a smart pointer. Here, it looks like you're sharing ownership of // the object by way of a raw pointer, which completely undermines the // purpose of the shared_ptr class (which is to manage shared ownership // by way of reference counting). // The typical thing to do here would be to return a shared_ptr object. return node.get();}
Author
Quote:Original post by SiCrane
Also, if you're using smart pointers, why return a dumb pointer from the function?
How exactly would I do that? Pass _nodes.back() by reference?
I think of the shared_ptr as a structure containing a pointer. So returning a shared_ptr itself would be returning a copy? Am I thinking about this incorrectly?
Author
Quote:Original post by jyk
A couple of additional comments:
*** Source Snippet Removed ***
Doesn't this break one of the rules in the boost documentation?
/* Avoid using unnamed shared_ptr temporaries to save typing; to see why this is dangerous, consider this example:*/void f(shared_ptr<int>, int);int g();void ok(){ shared_ptr<int> p(new int(2)); f(p, g());}void bad(){ f(shared_ptr<int>(new int(2)), g());}
Quote:Original post by mastodon
How exactly would I do that? Pass _nodes.back() by reference?
You wouldn't. You pass it back by value.
Quote:I think of the shared_ptr as a structure containing a pointer. So returning a shared_ptr itself would be returning a copy? Am I thinking about this incorrectly?
It would return a copy of the shared_ptr<>, but the underlying object pointed at is not copied.
Try this experiment:
#include <boost/shared_ptr.hpp>#include <iostream>int main(){ using namespace boost; shared_ptr<int> p1(new int(5)); shared_ptr<int> p2(p1); // copy std::cout << p1.get() == p2.get() << '\n'; return 0;}Quote:Original post by mastodonQuote:Original post by jyk
A couple of additional comments:
*** Source Snippet Removed ***
Doesn't this break one of the rules in the boost documentation?
*** Source Snippet Removed ***
jyk's code is perfectly safe. The point of the example in the boost documentation relates to exception safety.
Consider this call:
f(shared_ptr<int>(new int(2)), g());Here, the compiler may do things in this order:
int *temp1 = new int(2);int temp2 = g();shared_ptr<int> temp3(temp1);f(temp3, temp2);If g() throws an exception, you have a memory leak because temp1 never gets deleted.
By creating a shared_ptr<> separately, we avoid this situation:
shared_ptr<int> p(new int(2));f(p, g());Now there's no way that a memory leak will occur, even if g() throws an exception.
Quote:Original post by mastodonQuote:Original post by jyk
A couple of additional comments:
*** Source Snippet Removed ***
Doesn't this break one of the rules in the boost documentation?
*** Source Snippet Removed ***
I think the point was that the initialize method should really be a constructor and not to collapse it all into a temporary like the boost docs say not to do.
e.g.:
typedef boost::shared_ptr<StaticMeshNode> NodePtr;NodePtr SceneManager::addStaticMeshNode(const char* filename) { NodePtr node(new StaticMeshNode(filename, _resourceManager, _renderer, this)); _nodes.push_back(node); return node;}This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
