views:

718

answers:

8

Just a conceptual question that I've been running into. In my current project it feels like I am over-using the boost smart_ptr and ptr_container libraries. I was creating boost::ptr_vectors in many different objects and calling the transfer() method to move certain pointers from one boost::ptr_vector to another.

It is my understanding that it is important to clearly show ownership of heap allocated objects.

My question is, would it be desirable to use these boost libraries to create heap-allocated members that belong to an object but then use normal pointers to these members via get() when doing any processing.

For example... A game might have a collection of Tiles that belong to it. It might make sense to create these tiles in a boost::ptr_vector. When the game is over these tiles should be automatically freed.

However if I want to put these Tiles in a Bag object temporarily, should I create another boost::ptr_vector in the bag and transfer the Game's Tiles to the Bag via transfer() or should I create a std::vector<Tile*> where the Tiles*'s reference the Tiles in the Game and pass that to the Bag?

Thanks.

**Edit I should point out that in my example The Game would have a Bag object as a member. The Bag would only be filled with Tiles the game owns. So the Bag would not exist without the Game.

+9  A: 

You should only use owning smart pointers and pointer containers where there's clear transfer of ownership. It doesn't matter if the object is temporary or not - all that matters is whether it has ownership or not (and, therefore, whether the previous owner relinquishes ownership).

If you create a temporary vector of pointers just to pass it to some other function, and the original ptr_vector still references all those objects, there's no ownership transfer, and therefore you should use plain vector for the temporary object - just as you'd use a raw pointer to pass a single object from ptr_vector to a function that takes a pointer, but doesn't store it anywhere.

Pavel Minaev
A: 

boost::ptr_vector only serves to improve the semantics of using vectors of pointers. If, in your example, the original array could be destroyed while the temporary set of vectors is in use, then you should definitely be using a vector of shared_ptrs to prevent them from being deleted while still in use. If not, then a plain old vector of pointers may be appropriate. Whether you choose std::vector or boost::ptr_vector doesn't really matter, except in how nice the code that uses the vector looks.

Tim Sylvester
+2  A: 

In my experience, there are three main ownership patterns that crop up. I will call them tree, DAG and graph.

The most common is a tree. The parent owns its children, which in turn owns its children and so on. auto_ptr, scoped_ptr, bare pointers and the boost ptr_x classes are what you typically see here. In my opinion, bare pointers should generally be avoided as they convey no ownership semantics at all.

The second most common is the DAG. This means you can have shared ownership. The children a parent owns may also be the children of other children the parent owns. The TR1 and boost shared_ptr template is the main actor here. Reference counting is a viable strategy when you have no cycles.

The third most common is the full graph. This means that you can have cycles. There are some strategies for breaking those cycles and returning to a DAG at the cost of some possible sources of error. These are generally represented by TR1 or boost's weak_ptr template.

The full graph that can't be broken down into a DAG using weak_ptr is a problem that can't easily be solved in C++. The only good handlers are garbage collection schemes. They are also the most general, capable of handling the other two schemes quite well as well. But their generality comes at a cost.

In my opinion, you can't overuse the ptr_x container classes or auto_ptr unless you really should be using containers of objects instead of containers of pointers. shared_ptr can be overused. Think carefully about whether or not you really need a DAG.

Of course I think people should just be using containers of scope_ptrs instead of the boost ptr_x classes, but that's going to have to wait for C++0x. :-(

Omnifarious
A: 

In your game tiles example, my first instinct would be to create a vector<Tile>, not a vector<Tile*> or a ptr_vector. Then use pointers to the tiles as you please, without worrying about ownership at all, provided that the objects which hold the pointers don't outlive the vector of tiles. You've said it's only destroyed when the game ends, so that shouldn't be difficult.

Of course there may be reasons this is not possible, for instance because Tile is a polymorphic base class, and the tiles themselves are not all of the same class. But this is C++, not Java, and not every problem always needs dynamic polymorphism. But even if you do really need pointers, you can still make copies of those pointers without any ownership semantics, provided that the scope and storage duration of the objects pointed to is understood to be wider than the scope and duration of use of the pointer:

int main() {
    vector<Tile*> v;
    // fill in v, perhaps with heap objects, perhaps with stack objects.
    runGame(v);
}

void runGame(const vector<Tile*> &v) {
    Tile *firsttile = v[0];
    vector<Tile*> eventiles;
    eventiles.push_back(v[2]);
    eventiles.push_back(v[4]);
    // and so on. No need to worry about ownership, 
    // just as long as I don't keep any pointers beyond return.
    // It's my caller's problem to track and free heap objects, if any.
}
Steve Jessop
A: 

If Tiles are placed in the Bag and someone steals the Bag you lose all tiles. Therefore Tiles, althought temporarly, but they belong to the Bag for a short time. You should transfer ownership here, I suppose.

But the better opinion would be not to mess with ownership, because I don't see why it's needed in this particular case. If there's something behind the scene, go on, make your choice.

Pavel Shved
+1  A: 

Most likely, the solution you're looking for is

std::vector<Tile>

There's no need for pointers most of the time. The vector already takes care of memory managements of the contained objects. The tiles are owned by the game, aren't they? The way to make that explicit is to put the objects themselves in the game class -- the only cases where you typically need pointers and dynamically allocated individual objects is if you need 1) polymorphism, or 2) shared ownership.

But pointers should be the exception, not the rule.

jalf
+1  A: 

If the Game owns the tiles then the Game is responcable for there deltion.

It sounds like the bag never actually ownes the objects so it should not be responcable for deleting them. Thus I would use ptr_vector within the Game object. But use a std::vector in the bag.

Note: I would never let anybody using the bag retrieve a pointer to a tile from the bag. They should only be able to retrieve a referenceto the tile from the bag.

Martin York
A: 

I agree that using vector instead of jumping right into heap-allocated T is a good instinct, but I can easily see Tile being a type for while copy construction is expensive, which might preclude vector's growth strategy being practical. Of course, that might be best solved with a list, not a vector...

me22