Add boiler point for state machine #31
Conversation
Mchan2003
force-pushed
the
Michael-CLI-FSM
branch
from
3e30222 to
536a194
Compare
PhazonicRidley
left a comment
PhazonicRidley
left a comment
There was a problem hiding this comment.
Overall good! One small design question, is there a reason you are using raw pointers for your states? Do they need to stay alive after exiting? If so, is there a reason you are using raw pointers over smart pointers
PhazonicRidley
left a comment
PhazonicRidley
left a comment
There was a problem hiding this comment.
Very good! Just some final tweaks :)
| std::optional<std::reference_wrapper<State>> StateMachine::get_current_state() | ||
| { | ||
| if (m_current_state) { | ||
| return std::ref(**m_current_state); |
There was a problem hiding this comment.
Do not use the deref operator directly on an optional, this is UB. Please use one of the following methods to safely get access to what optional is wrapping: https://en.cppreference.com/w/cpp/utility/optional.html
If you wish to just throw an exception if its nullopt, use .value()
| expect(actual_state.has_value()) << "A State is initialized"; | ||
|
|
||
| State& actual_state_ref = actual_state.value().get(); | ||
| expect(typeid(actual_state_ref) == typeid(UserInputState)) |
There was a problem hiding this comment.
Please add that % in front of your boolean expressions. This is so, if it errors it shows you the actual expression that was not true. Without that it will just show false
Example: https://godbolt.org/z/KMPr7ojfY
|
Whats the purpose of the state machine? Each state looks like its just another step in the process. Whats the need for polymorphism and interfaces? Why not just write a function that performs the steps.
I'm not sure where there are states here. |
Fixes #33