State Design Pattern - Don't want to delete this pointer in the member class
I am using the example below to implement the stateful design pattern.
https://sourcemaking.com/design_patterns/state/cpp/1
I do not want to delete * this pointer inside the member class because it is unsafe (I will call another member function after deleting).
class ON: public State
{
public:
ON()
{
cout << " ON-ctor ";
};
~ON()
{
cout << " dtor-ON\n";
};
void off(Machine *m);
};
class OFF: public State
{
public:
OFF()
{
cout << " OFF-ctor ";
};
~OFF()
{
cout << " dtor-OFF\n";
};
void on(Machine *m)
{
cout << " going from OFF to ON";
m->setCurrent(new ON());
delete this; // <<< This line looks suspect and unsafe
}
};
void ON::off(Machine *m)
{
cout << " going from ON to OFF";
m->setCurrent(new OFF());
delete this; // <<< This line looks suspect and unsafe
}
Is there a better approach to implementing the government design pattern? I was thinking about using singletons, but I want to avoid using singletons.
Respectfully,
source to share
"... but I want to avoid using singletones."
This is probably one of their rare valid use cases, since states should be stateless themselves (which doesn't mean they don't need an instance) and are available simultaneously regardless of the state machine's current state. So this means that you really only need one instance of the state at a time.
The example shown on the website mentioned also gives you an unnecessary performance hit at the cost of new
and delete
whenever the state changes, which can be avoided to have persistent instances static
for the states.
In fact, you might consider providing state instances, as with the Flyweight design pattern , which essentially boils down to having Singleton state instances.
But it depends. UML state graphs actually allow stateless states (as composite states with history attributes or active states).
Take a look at my STTCL Template Library Concept Paper , it explains some of the aspects and design solutions I used to develop this template library and how it can be used correctly.
Rest assured I have none ;-) delete this;
.
I would say that the example the website provides at the moment is really poorly designed and not recommended at all 1 .
Use delete this
is unsuitable, dangerous and unacceptable as you mentioned.
When using Singleton, there is if you have a valid use case.
1) Unfortunately, we will notice this, as I have used it as a "link" for many design-patterns here.
source to share
Design used in your link:
- the state machine keeps track of a pointer to the current state.
- the current state is responsible for setting a new state when the occcurs transition
- it does this by creating a new state and telling the machine to change the pointer
- Once this is done, he will commit suicide by removing himself.
As explained here , this can work if extra care is taken. Your code meets the prerequisites.
An alternative would be that the state machine removes the current state in setCurrent()
. But this is worse: as soon as the machine has deleted an object, this object no longer exists, so when it setCurrent()
returns, you are de facto in the same (dangerous) situation as before (c delete this
), with the important difference that it was not would be obvious.
Edit:
If you feel uncomfortable with this design, you can choose an option:
class Machine
{
class State *current; // current state
class State *nextstate; // null, unless a state transition was requested
void check_transition(); // organise switch to nextstate if transition is needed
public:
Machine();
void setCurrent(State *s)
{
nextstate = s; // only sets the info about nextstate
}
void set_on();
void set_off();
};
In this case, the state-machine functions should be slightly updated:
void Machine::set_on()
{
current->set_on(this); // as before
check_transition(); // organise the transition if the transition was requested
}
Then the state transition will be controlled by a state machine:
void Machine::check_transition() {
if (nextstate) {
swap (current, nextstate);
delete nextstate; // this contains the former current
nextstate = nullptr;
}
}
The machine organizes the removal of unused states. Note that this approach will allow you to use shared_ptr instead of raw pointers.
Here's a small online proof of concept .
By the way: the destructor of the machine must also remove the current and next state to avoid leaking. And in any case, the state destructor must be defined virtual
source to share