Open AleksandrHovhannisyan opened 4 years ago
@kroseberry Glad you figured it out! The reason light = new Light()
works in my TEST_METHOD_INITIALIZE
is because light
is declared as a private member variable as part of this test class/namespace.
Circular dependency is not good to maintain a good and clean architecture? Essentially the state class and light class are closely coupled. Any idea how to improve?
@lz89 In this case, the circular dependency is a pretty classic characteristic of the state design pattern. More on that here: https://sourcemaking.com/design_patterns/state. Specifically, see the Check list
section.
Hi, Aleksandr
I'm trying to use your tutorial as a template for a FSM and I don't know how I can access the "enter" or "exit" methods using the light pointer. Should I create "enter" and "exit" methods in Light.h and Light.cpp?
@iandownie07 Hey! Just to clarify, the enter
and exit
methods are part of the LightState
interface, meaning they are only used by the light states themselves and not the Light
itself. The Light
should not be aware of what state it is in; the idea is that the Light
holds a reference to its current state (some state), and it passes itself to that state so that the state is in turn aware of the object with which it's associated. The states singleton then manage its own lifecycle—what happens when you enter/exit that particular state, as well as what should happen while you're in that state. I hope that clears things up!
Thanks for the reply, Aleksandr.
I worked out a few judiciously placed currentState->enter(this) or light->exit() got me what I wanted. The code has been really useful - thanks for making it available.
Hello :) I am quite new to some concepts used here Edited (because i solved my initial problem) : how would i get the name of the current State? the getCurrentState() function returns something like this: 0x55ca51a2e2d8 Thanks for any help
Thanks for the very useful article! I used it to re-create one of the examples in the Pragmatic Programmer, outputting the parts of a string that are inside quotes.
https://gist.github.com/cjappl/493ee614995c1b9a80cc73b1fb3f7ca6
Really appreciate all the work you did on the article, really helped me make this concrete.
Is the code from this tutorial (along with tests) available somewhere?
@berger156 No, but I could put it up on GitHub or as a gist later if I have time. An earlier version of this article showed how to set up unit tests, but I felt that was distracting/tangential to the main subject, so I removed it.
@AleksandrHovhannisyan: I had seen the discussion of unit tests above, and that's what caught my interest. I find such examples instructive for learning how the code operates.
Hello @AleksandrHovhannisyan , I'm in a learning process and i try to implement first method, but I can't figured out how my main should look, or how I can test it, can you give me some help ? thanks for example!
@mduduAC Something like this should work:
#include "Light.h"
int main() {
Light::Light light = new Light::Light();
light->toggle();
return 0;
}
A previous version of this article showed how to set up unit tests in Visual Studio, but I later removed that section to keep the article focused.
I can update the article to show a sample main file, though—thanks for the feedback!
Thanks Alexandr. This was very helpful. Where would you implement functionality that needs to be accessible for all states? The Light class seems to be the most logical place at first sight. Or would you advise something else?
@mvandeg-git Yes, I would put any shared functionality in the base class. The FSM pattern does not strictly require that you only use these three methods in your state classes (and the method names are, of course, also up to you).
Don't you need to place the virtual keyword in front of void enter(Light light) {}, void toggle(Light light); void exit(Light* light) {} etc in ConcreteLightStates.h ?
There is a discussion on it here: https://stackoverflow.com/questions/52708430/implicit-virtual-vs-explicit-virtual-in-inherited-classes-at-compile-time And I got warnings without on some but not all of my compilers.
@frog1776 You only need the virtual
keyword on the LightState
class to mark it as an abstract class (i.e., one that cannot be instantiated). The concrete subclasses don't need the keyword—if you add it, then you won't be able to create a new LightOff
, LightLow
, etc.
Thanks Alexandr. Apologies for the late reply. This was very helpful and the project for which I used this FSM implementation is very stable.
On Wed, 13 Sept 2023, 02:00 Aleksandr Hovhannisyan, < @.***> wrote:
@mvandeg-git https://github.com/mvandeg-git Yes, I would put any shared functionality in the base class. The FSM pattern does not strictly require that you only use these three methods in your state classes (and the method names are, of course, also up to you).
— Reply to this email directly, view it on GitHub https://github.com/AleksandrHovhannisyan/aleksandrhovhannisyan.com/issues/33#issuecomment-1716737335, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUI4NLAQYGOAIZDAEHVUS7DX2DZRHANCNFSM4NIP5APQ . You are receiving this because you were mentioned.Message ID: <AleksandrHovhannisyan/aleksandrhovhannisyan.com/issues/33/1716737335@ github.com>
Thanks so much for posting this, it was really helpful! How would you implement this last method based on OOP using a Hierarchical State Machine that has several states and substates. The problem I have is that I when I enter the substate of a state, I will exit the state.
Say I have state A and B. And A has substates AA and AB. When I enter A and then enter AA, AA executes enter() and A executes exit(). I would only want to execute A exit() when I transition to B not to AA or AB.
/ \
A B
/ \
AA AB
Thanks in advance!
I tried to adapt this pattern to my own problem but keep getting the following linking error:
ld: Undefined symbols: Context::pressP(), referenced from: _main in main.cpp.o Context::pressS), referenced from: _main in main.cpp.o clang: error: linker command failed with exit code 1 (use -v to see invocation)
This is really driving me nuts & it would be great if someone points me toward a solution.
There are also a couple of things i don't understand in the given code: 1) For the concrete states there are declarations of the copy constructors & the copy assignement operators but no definitions. 2) In Light.h & Lightstate.h : why do i need to forward declare classes when i already included the respective headers. Doesn't the headers include the full class-definition anyways?
Anyways, thanks a lot for this well written tut.
@kbas2910 Thanks for commenting. It's hard to say what could be causing your issue without seeing any code. I recommend using an IDE like Visual Studio that provides debugging tools or your favorite text editor with C++ LSP enabled.
For the concrete states there are declarations of the copy constructors & the copy assignement operators but no definitions.
The only time you need to provide a definition for a declared function in C++ is if you intend to use that function (which is the case the majority of the time). In this particular case, declaring the copy constructor and assignment operator private serves one purpose: To prevent us from calling them! And if we can't call them, then we don't have to define them.
In
Light.h
&Lightstate.h
: why do i need to forward declare classes when i already included the respective headers. Doesn't the headers include the full class-definition anyways?
LightState
depends on Light
, and Light
depends on LightState
, so we have circular includes/dependencies. In these types of situations, you need to provide forward-declarations to help the compiler resolve the circular includes, or else your code won't compile. The best way to learn/understand concepts like this is to try to break the code—remove the forward declarations and see what happens.
Thanks a lot for your effort. In case someone is willing to invest even more time on my silly problems i am posting some code below. Please ignore/ delete, if inapropriate.
This is supposed to be the basis of some kind of midi-sequencer.
Upon Buttonpresses (Play or Stop) it is supposed to change state. For example: PlayingState -> Press Play -> PausedState.
I have also implemented a timer with chrono library that sends timing events ( 'tics' ) and some other logic but the problem seems to lie in my implementation of the state pattern. I can instantiate a Sequencer in main but when i call a method compilation fails with a linker error.
sequencerState2.hpp
#pragma once
#include <iostream>
#include "sequencer2.hpp"
class Sequencer ;
class SequencerState {
public:
virtual ~SequencerState() {}
virtual void pressPlay(Sequencer* s) = 0 ;
virtual void pressStop(Sequencer* s) = 0 ;
// virtual void tickReceived(Sequencer* s) = 0 ;
} ;
concreteStates2.hpp
#pragma once
#include "sequencerState2.hpp"
#include "sequencer2.hpp"
class IdleState : public SequencerState {
public:
void pressPlay(Sequencer* s) ;
void pressStop(Sequencer* s) ;
// void tickReceived(Sequencer* s) override ;
static SequencerState& getInstance() ;
private:
IdleState() {}
IdleState(const IdleState& other) ;
IdleState& operator=(const IdleState& other) ;
} ;
class PlayingState : public SequencerState {
public:
void pressPlay(Sequencer* s) ;
void pressStop(Sequencer* s) ;
// void tickReceived(Sequencer* s) override ;
static SequencerState& getInstance() ;
private:
PlayingState() {}
PlayingState(const PlayingState& other) ;
PlayingState& operator=(const PlayingState& other) ; //
} ;
class PausedState : public SequencerState {
public:
void pressPlay(Sequencer* s) ;
void pressStop(Sequencer* s) ;
// void tickReceived(Sequencer* s) override ;
static SequencerState& getInstance() ;
private:
PausedState() {}
PausedState(const PausedState& other) ; // COPY CONSTRUCTOR (NEEDED? IMPLEMENTATION NEEDED?)
PausedState& operator=(const PausedState& other) ;
} ;
concreteStates2.cpp
#include "concreteStates2.hpp"
#include <iostream>
void IdleState::pressPlay(Sequencer* s) { s->transitionStateTo(PlayingState::getInstance()) ; }
void IdleState::pressStop(Sequencer* s) { std::cout << "debug info\n" ; }
// void IdleState::tickReceived(Sequencer* s) { std::cout << "tic\n " ; }
SequencerState& IdleState::getInstance() {
static IdleState singleton ;
return singleton;
}
void PlayingState::pressPlay(Sequencer* s) { s->transitionStateTo(PausedState::getInstance()) ; }
void PlayingState::pressStop(Sequencer* s) { s->transitionStateTo(IdleState::getInstance()) ; }
// void PlayingState::tickReceived(Sequencer* s) { std::cout << "executeCommand[m_currentPosition]\n" ; }
SequencerState& PlayingState::getInstance() {
static PlayingState singleton ;
return singleton;
}
void PausedState::pressPlay(Sequencer* s) { s->transitionStateTo(PlayingState::getInstance()) ; }
void PausedState::pressStop(Sequencer* s) { s->transitionStateTo(IdleState::getInstance()) ; }
// void PausedState::tickReceived(Sequencer* s) { std::cout << "tic\n" ; }
SequencerState& PausedState::getInstance() {
static PausedState singleton ;
return singleton;
}
sequencer2.hpp
#pragma once
#include <iostream>
#include <chrono>
using ms = std::chrono::milliseconds ;
#include "sequencerState2.hpp"
class SequencerState ;
class Sequencer {
private:
SequencerState* m_sequencerState {} ;
std::string m_commands[16] = {"01", "02", "03", "04", "05", "06", "07", "08", "09", "10", "11", "12", "13", "14", "15", "16" } ;
int m_currentPosition { 0 } ;
ms m_BPM {} ;
public:
Sequencer() {}
inline SequencerState* getCurrentSequencerState() const { return m_sequencerState ;}
void transitionStateTo(SequencerState& newSequencerState) ;
void pressPlay() ;
void pressStop() ;
// void tickReceived() ;
void run() ;
};
sequencer2.cpp
#include "sequencer2.hpp"
#include "concreteStates2.hpp"
#include <chrono>
using Clock = std::chrono::system_clock ;
extern std::atomic<bool> quitButtonPressed ;
Sequencer::Sequencer() {
m_sequencerState = &IdleState::getInstance() ;
}
void Sequencer::transitionStateTo(SequencerState& newState) {
m_sequencerState = &newState ;
}
void Sequencer::pressPlay() {
m_sequencerState->pressPlay(this) ;
}
void Sequencer::pressStop() {
m_sequencerState->pressStop(this) ;
}
/*
void tickReceived() {
this->m_sequencerState->tickReceived() ;
}
*/
void Sequencer::run() {
std::chrono::time_point<Clock> currentTime , lastTime ;
lastTime = Clock::now() ;
std::cout << "'S' -> STOP \n'P' -> PLAY\n'Q' -> QUIT PROGRAM\n" ;
while(true) {
currentTime = Clock::now() ;
if(currentTime-lastTime >= m_BPM) {
// m_sequencerState->tickReceived() ;
std::cout << "tic\n" ;
lastTime = Clock::now() ;
}
if(quitButtonPressed) { break; }
}
}
main
#include <iostream>
#include <thread>
#include "sequencer2.hpp"
static std::atomic<bool> quitButtonPressed { false } ;
int main() {
Sequencer S ;
S.pressPlay() ; // -> LINKER ERROR
return 0 ;
}
@mduduAC Something like this should work:
#include "Light.h" int main() { Light::Light light = new Light::Light(); light->toggle(); return 0; }
A previous version of this article showed how to set up unit tests in Visual Studio, but I later removed that section to keep the article focused.
I can update the article to show a sample main file, though—thanks for the feedback!
Hello @AleksandrHovhannisyan Thank you for posting this article FMS in C++. would you mind posting the unit tests setup in Visual Studio. I really would appreciate that. Ed.cali1960@gmail.com
@EdCantillo2017 I removed the unit testing code from my article a long time ago to keep it focused on the main topic. I'll see if I can dig up the diff in my commit history, but I think I moved/renamed the file at some point without git mv
.
I would recommend following Microsoft's documentation here if you want to set it up: https://learn.microsoft.com/en-us/visualstudio/test/writing-unit-tests-for-c-cpp?view=vs-2022.
Very interesting tutorial. I'm not familiar with the Visual Studio Test Framework. How would you call this from main()? I tried:
But got the error: 'light' was not declaredin this scope; did you mean 'Light'? But Light didnt seem right either.
Any help would be greatly appreciated.
Solved myself (I was close): I'm using Eclipse. This will work in any main():