Sign in to follow this  
Angelic Ice

List of Smart Pointers to Objects

Recommended Posts

Hello forum!

 

I was wondering if this bad design:

 

I have a class that handles components of my game. One of these components is the state component.

It holds a list of smart pointers to states. These states derive from their base class. The handler of this component iterates over the list and calls all the active states update methods. If a state is paused, it will be skipped.

 

Now, how would I handle the situation of a state being no longer required in order to free the RAM?

Should I just delete the state? That is what I feel like is the only possibility.

 

Is that okay? Is this bad design?

What other ways would be better?

Share this post


Link to post
Share on other sites

There are many kinds of "smart" pointer, all of which do very different things and have very different semantics and implications regarding ownership (which seems to be the crux of your issue here). So what kind of smart pointer are you thinking about here?

 

A std::unique_ptr would probably be a reasonable choice, as it sounds like the component is the authoritative owner of the states and controls their lifetime. In that case you just reset() the pointer to release the underlying object.

Share this post


Link to post
Share on other sites

So it is completely legit to have this authoritative owner which controls lifetime?

 

Yes, and it's usually good. Having clear rules and conventions about who owns what leads to fewer bugs, generally speaking, and easier-to-reason-about interfaces.

Share this post


Link to post
Share on other sites

So it would be bad if the state wants to delete itself?

 

For example, the state has its own key-events. Now, a key has been pressed that will create a new state. The old state is now pointless.

However, the state does not know its smart pointer in the list. How would this work out?
Or should it be avoided?

Share this post


Link to post
Share on other sites

So it would be bad if the state wants to delete itself?

 

For example, the state has its own key-events. Now, a key has been pressed that will create a new state. The old state is now pointless.

However, the state does not know its smart pointer in the list. How would this work out?
Or should it be avoided?

 

One option would be to set a flag for the object saying it is no longer required. The code that loops through calling Update on the components can then check that flag and remove the component if necessary. You mentioned you already have the ability to pause a state, this would be similar.

 

if(!component->IsAlive()) remove from list.

Share this post


Link to post
Share on other sites

So it would be bad if the state wants to delete itself?

 

Depends. If you're talking about "delete this;" than that is almost certainly a bad idea. If you're talking about knowing that the state has completed its work and should become inactive permanently, that's another thing altogether. You already have a system by which a state can make itself active or not; this sets a flag which the owner of the state queries when doing updates. If you want to support "permanent deactivation," you can simply create a similar flag and have the owner collect all states that marked themselves as permanently deactivated during an update and delete them itself.

 

Personally it seems like a weird thing for a state to do, but maybe it makes more sense in the context of your specific implementations. 

 

 

For example, the state has its own key-events. Now, a key has been pressed that will create a new state. The old state is now pointless.

 

 

 
This makes a little more sense; this is just transitioning to a new state, which you can also communicate via some flags or calls to the owner. You may not actually want to destroy the original state here (what if the user wants to come back to it and see everything in the same spot?) though.

Share this post


Link to post
Share on other sites

Ah! I thought that flags are a rather ugly way of handling, that was just my feeling though, as I had this idea of deleting them directly.

 

This makes a little more sense; this is just transitioning to a new state, which you can also communicate via some flags or calls to the owner. You may not actually want to destroy the original state here (what if the user wants to come back to it and see everything in the same spot?) though.

That is why I have my pause state, but some states really have no further purpose and just serve as brief GUI interface and so on. Therefore, I want to get rid of them.

For example a pause-screen.

 

if(!component->IsAlive()) remove from list.

This might be for another thread, but I thought getter/setter should be avoided by using methods that just do things instead of requesting information? This might be more of a religious thing - there were some people talking about it. It feels hard to avoid getter/setter sometimes, though.

Share this post


Link to post
Share on other sites

If you can't query other objects, it makes it very awkward to construct any sort of useful program. You'd have to have a function inside the state that calls out and tells its owner to delete it, which couldn't do that immediately because the state is still alive at that point, so it would have to make a note and do that later. Ugly.

 

Making an object full of getters and setters for each field is usually a bad idea because that leaves you with no real encapsulation. But there's nothing wrong with an object being able to report some aspect of its state.

Share this post


Link to post
Share on other sites

 

if(!component->IsAlive()) remove from list.

This might be for another thread, but I thought getter/setter should be avoided by using methods that just do things instead of requesting information? This might be more of a religious thing - there were some people talking about it. It feels hard to avoid getter/setter sometimes, though.

 

Very few things are right or wrong or best or worse, it all depends on your unique problem that you wish to solve. You could try to have the component remove itself from the list (access parent, get list of components, find self in list, remove self) but that is really, really asking for trouble. The alternative is then to have something else remove it, preferable the object that owns in (your entity). At that point you need some way to communicate to the parent that it should remove the component and the best way to do that is, in my opinion, with a flag such as this. You really want to avoid doing it while the component is being updated so setting a flag allows it to be removed at a more appropriate time. You don't have to use a getter type method but I wouldn't avoid using it just for the sake of avoiding using it. If/when you start getting 'a lot' of getters/setters then I would start questioning things.

Share this post


Link to post
Share on other sites

Thanks a lot for all the tips!

 

When talking about a flag, what would be a good idea in this case?

The states are all deriving from their base featuring some virtual voids to let the controlling component control them.

 

If I would go with an enum, featuring all the states like pause/active/done, ...

Where should I place this? It feels weird to have it in the base class but to have to refer to the base class just in order to access them.

However, I have to define the enum somewhere.

 

So, should I go with a different way of flagging?

Share this post


Link to post
Share on other sites

I don't see a problem with having such flags/methods in your base class interface if it's something that makes sense with how the system as a whole works. IsPaused()/IsAlive() seem perfectly good things to have to me. You could have a generic GetFlags() method and an enum with possible flags. Personally, if it's only 1 or 2 things that you care about (is the component paused, is the component dead) then I would use a method for it.

Share this post


Link to post
Share on other sites

If it suits you then it might be an ok design.

If it REALLY suits you, it's a good design.

 

Usually there's nothing wrong with holding a list of raw pointers, it just means the list doesn't control the memory.

If you have any smart memory managment mechanism (I understand you don't) then a raw list is fine (because you don't deallocate when you remove the item but when you choose to deallocate it).

A list of unique ptrs is also fine because the meaning is the list contains the memory and nobody else controls it.

So removing an item means deallocation of the resource.

 

 

About your flag question:

Put it in the most reusable class you find, if you find yourself reusing states, then create a base class for it.

How you imlement states is a different question, if there are several states enum is fine.

If the enum might grow into 20 different states then find a different approach. 

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this