Question about using boost::shared_ptr in an std::list or vector

This topic is 3797 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

I'm writing a scene manager which has an std::list of all of the nodes. Here is some code:
std::list< boost::shared_ptr<SceneNode> > _nodes; // List of all scene nodes.

boost::shared_ptr<StaticMeshNode> node(new StaticMeshNode());
node.get()->initialize(filename, _resourceManager, _renderer, this);
_nodes.push_back(node);

return node.get();
}


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

Share on other sites
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.

Share on other sites
Also, if you're using smart pointers, why return a dumb pointer from the function?

Share on other sites
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.

Share on other sites

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();}

Share on other sites
Quote:
 Original post by SiCraneAlso, 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?

Share on other sites
Quote:
 Original post by jykA 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());}

Share on other sites
Quote:
 Original post by mastodonHow 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;}

Share on other sites
Quote:
Original post by mastodon
Quote:
 Original post by jykA 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.

Share on other sites
Quote:
Original post by mastodon
Quote:
 Original post by jykA 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;}

1. 1
2. 2
Rutin
18
3. 3
4. 4
5. 5

• 14
• 12
• 9
• 12
• 37
• Forum Statistics

• Total Topics
631432
• Total Posts
3000040
×