Thread: State design pattern - can't find an error

  1. #1
    Registered User
    Join Date
    Mar 2016
    Posts
    36

    Post State design pattern - can't find an error

    Hello guys, I am trying to implement a State Design pattern, but I am stuck with it since I can't get my code compiled, and can't find solution to it

    Error happens in this part of the code, while trying to call method setState
    Code:
    void StartTransmitted::nextState(I2CStateContext* currentContext)
    {
        cout << "Start Transmitted is running .." << endl;
        this->sendSignal();
        // this is where error occours
        currentContext->setState(new AdrWTransACKRec());
    }



    StartTransmitted.h
    Code:
    #pragma once
    #ifndef _START_TRANSMITTED_H_
    #define _START_TRANSMITTED_H_
    
    #include "I2CState.h"
    
    class StartTransmitted : public I2CState
    {
        public:
            void nextState(I2CStateContext* currentContext);
    
        private:
            void sendSignal(void);
    };
    
    #endif


    StartTransmitted.cpp
    Code:
    #include "StartTransmitted.h"
    #include "AdrWTransACKRec.h"
    
    
    void StartTransmitted::nextState(I2CStateContext* currentContext)
    {
        cout << "Start Transmitted is running .." << endl;
        this->sendSignal();
        // This is line where error happens
        currentContext->setState(new AdrWTransACKRec());
    
    }
    
    void StartTransmitted::sendSignal(void)
    {
        cout << "Sending start signal" << endl;
    }
    I2CState.h
    Code:
    #pragma once
    #ifndef _I2C_STATE_H_
    #define _I2C_STATE_H_
    
    #include <iostream>
    
    class I2CStateContext;
    
    using namespace std;
    
    class I2CState
    {
        public:
            virtual void nextState(I2CStateContext* currentContext);
    
        public:
            virtual void sendSignal(void);
            
    };
    
    #endif
    I2CStateContext.h
    Code:
    #include "I2CStateContext.h"
    
    I2CStateContext::I2CStateContext()
    {
        //this->currentState = new State();
    }
    
    void I2CStateContext::setState(I2CState* newState)
    {
        this->currentState = newState;
    }
    
    void I2CStateContext::nextState()
    {
        currentState->nextState(this);
    }
    I2CStateContext.cpp
    Code:
    #pragma once
    #ifndef _I2C_CONTEXT_H_
    #define _I2C_CONTEXT_H_
    
    #include "I2CState.h"
    
    class I2CStateContext
    {
        private:
            I2CState* currentState;
        
        public:
            I2CStateContext();
            void setState(I2CState* newState);
            void nextState();
    
    };
    
    #endif
    Attached Images Attached Images State design pattern - can't find an error-error-jpg 
    Last edited by ZeroesAndOnes; 03-26-2017 at 02:22 PM.

  2. #2
    Guest
    Guest
    I presume you made a mistake in your post when naming the files? I see I2CState.h twice. I think you might be missing a declaration of I2CStateContext. The one in your I2CState.cpp should be inside a header file that the other units can see.

  3. #3
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    Quote Originally Posted by Guest View Post
    I presume you made a mistake in your post when naming the files? I see I2CState.h twice. I think you might be missing a declaration of I2CStateContext. The one in your I2CState.cpp should be inside a header file that the other units can see.
    Yes I mistakenly written I2CState.h instead of I2CStateContext.h fixed it now

    I2CState is just an interface,
    So I don't need .cpp file for it?

    Code:
    #pragma once
    #ifndef _I2C_STATE_H_
    #define _I2C_STATE_H_
    
    #include <iostream>
    
    class I2CStateContext;
    
    using namespace std;
    
    class I2CState
    {
        public:
            virtual void nextState(I2CStateContext* currentContext) = 0;
    
        public:
            virtual void sendSignal(void) = 0;
            
    };
    Last edited by ZeroesAndOnes; 03-26-2017 at 02:31 PM.

  4. #4
    Guest
    Guest
    Is is possible that you mixed up I2CStateContext.cpp and I2CStateContext.h as well? It's late and I'm tired, so I might be missing something obvious here.

    Anyway, this code...
    Code:
    class I2CStateContext
    {
        private:
            I2CState* currentState;
        
        public:
            I2CStateContext();
            void setState(I2CState* newState);
            void nextState();
    };
    ... should be inside I2CStateContext.h and that header included in the translation units that need it.

    For instance StartTransmitted.cpp includes StartTransmitted.h includes I2CState.h which declares class I2CStateContext;. You cannot use that class if just the forward declaration is known at that point.
    Last edited by Guest; 03-26-2017 at 02:56 PM.

  5. #5
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    I2CStateContex.h
    This code is in I2CStateContext.h
    Code:
    #pragma once
    #ifndef _I2C_STATE_CONTEXT_H_
    #define _I2C_STATE_CONTEXT_H_
    
    #include "I2CState.h"
    
    class I2CStateContext
    {
        private:
            I2CState* currentState;
        
        public:
            I2CStateContext();
            void setState(I2CState* newState);
            void nextState();
    
    };
    
    #endif

    Also I2CStateContext is included in I2CState which is an interface as I mentioned

    I2CState.h

    Code:
    #pragma once
    #ifndef _I2C_STATE_H_
    #define _I2C_STATE_H_
    
    #include <iostream>
    #include "I2CStateContext.h"
    
    using namespace std;
    
    class I2CState
    {
        public:
            I2CState();
            virtual void nextState(I2CStateContext* currentContext) = 0;
    
        public:
            virtual void sendSignal(void) = 0;
            
    };
    
    #endif
    I think problem might be that I2CState Cannot be compiled if "I2CStateContext.h" is not included and vice-versa.

  6. #6
    Registered User
    Join Date
    Jun 2015
    Posts
    1,643
    Looks like you need forward references for the classes. Put "class I2CState;" before the declaration of I2CStateContext to inform it of the existence of that class. Also put "class I2CStateContext;" before the declaration of I2CState.

    And never put a "using namespace std" in a header file! It should be the user's choice to dump the entire std namespace if he so desires.

  7. #7
    Guest
    Guest
    And I think in StartTransmitted.cpp you can then include the I2CStateContext.h header with the full declaration, which should allow you to work with the class pointer.

  8. #8
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    Quote Originally Posted by algorism View Post
    Looks like you need forward references for the classes. Put "class I2CState;" before the declaration of I2CStateContext to inform it of the existence of that class. Also put "class I2CStateContext;" before the declaration of I2CState.

    And never put a "using namespace std" in a header file! It should be the user's choice to dump the entire std namespace if he so desires.
    I actually put "class I2CStateContext" before declaration of I2CState but your post got me idea to put "class I2CState" before I2CStateContext declaration and it compiled.

    About using namspace std, well sure I am using Visual studio just to check if my code is working, real code will be written in AVR-GCC for a microcontroller.

    However I have another question about implementing State Design pattern,
    my class I2CStateContext where states are changed based on the interrupt, however I created

    class I2CMode interface which uses State Design pattern,
    - Idle, Instantiating, Master Transmitter ... and other modes are implemeting this interface

    One of the operating modes
    Code:
    public class MasterTransmitter implements I2CMode 
    {
    
        public void nextState(I2CModeContext currentMode)
        {
            currentMode.setState(new Idle());
        }
        
        public void handle()
        { 
            System.out.println("");
            System.out.println("Master Transmitter MODE...");
        }
        
    }

    I2CMode Interface
    Code:
    public interface I2CMode 
    {
        public void nextState(I2CModeContext currentContext);
        public void handle();
    
    }
    So basically what I did is I used composition inside I2CStateContext class

    Code:
    class I2CStateContext
    {
        private:
            I2CState* currentState;
            I2CModeContext* currentModeContext;
    
        public:
            I2CStateContext();
            void setState(I2CState* newState);
            void nextState();
    
    };
    Would it be wise to use context inside of another context?

    I2CStates defines I2CModes (except for the start of the program when I2CMode is by default Idle), so It was logical to me to put a
    Code:
    I2CModeContext* currentModeContext
    inside of
    Code:
    class I2CStateContext
    Sorry for the part of code being written in Java, since I couldn't get code compiled I simply switched to Java.

  9. #9
    Registered User
    Join Date
    Jun 2015
    Posts
    1,643
    I can't really understand what you are asking. It seems like it might be overly-complicated since the state design pattern is pretty simple. I have no idea what your I2CModeContext is for or what you mean by "I2CStates defines I2CModes".

    Instead of talking about bits and pieces of code, maybe you could describe in general what you are trying to achieve.

    EDIT: This is the only sense I can make out of your problem. I'm no C++ expert, so it may need some work.
    Code:
    #include <iostream>
    
    class I2CStateContext;
    
    class I2CState {
    public:
        virtual void nextState(I2CStateContext& context) = 0;
        virtual void handle() = 0;
        virtual ~I2CState() { }
    };
    
    class I2CState_Idle : public I2CState {
        void nextState(I2CStateContext& context);
        void handle();
    };
    
    class I2CState_MasterTransmitter : public I2CState {
        void nextState(I2CStateContext& context);
        void handle();
    };
    
    class I2CStateContext {
    private:
        I2CState* state;
    public:
        I2CStateContext() : state(new I2CState_Idle()) {}
        void setState(I2CState* newState) {
            delete state;
            state = newState;
        }
        void nextState() { state->nextState(*this); }
        void handle() { state->handle(); }
        ~I2CStateContext() { delete state; }
    };
    
    void I2CState_Idle::nextState(I2CStateContext& context) {
        context.setState(new I2CState_MasterTransmitter);
    }
    
    void I2CState_Idle::handle() { 
        std::cout << "Idle Mode\n";
    }
    
    void I2CState_MasterTransmitter::nextState(I2CStateContext& context) {
        context.setState(new I2CState_Idle);
    }
    
    void I2CState_MasterTransmitter::handle() { 
        std::cout << "Master Transmitter Mode\n";
    }
    
    int main() {
        I2CStateContext context;
        for (int i = 0; i < 10; i++) {
            context.handle();
            context.nextState();
        }
        return 0;
    }
    Last edited by algorism; 03-26-2017 at 04:57 PM.

  10. #10
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    Quote Originally Posted by algorism View Post
    I can't really understand what you are asking. It seems like it might be overly-complicated since the state design pattern is pretty simple. I have no idea what your I2CModeContext is for or what you mean by "I2CStates defines I2CModes".

    Instead of talking about bits and pieces of code, maybe you could describe in general what you are trying to achieve.

    EDIT: This is the only sense I can make out of your problem. I'm no C++ expert, so it may need some work.
    Code:
    #include <iostream>
    
    class I2CStateContext;
    
    class I2CState {
    public:
        virtual void nextState(I2CStateContext& context) = 0;
        virtual void handle() = 0;
        virtual ~I2CState() { }
    };
    
    class I2CState_Idle : public I2CState {
        void nextState(I2CStateContext& context);
        void handle();
    };
    
    class I2CState_MasterTransmitter : public I2CState {
        void nextState(I2CStateContext& context);
        void handle();
    };
    
    class I2CStateContext {
    private:
        I2CState* state;
    public:
        I2CStateContext() : state(new I2CState_Idle()) {}
        void setState(I2CState* newState) {
            delete state;
            state = newState;
        }
        void nextState() { state->nextState(*this); }
        void handle() { state->handle(); }
        ~I2CStateContext() { delete state; }
    };
    
    void I2CState_Idle::nextState(I2CStateContext& context) {
        context.setState(new I2CState_MasterTransmitter);
    }
    
    void I2CState_Idle::handle() { 
        std::cout << "Idle Mode\n";
    }
    
    void I2CState_MasterTransmitter::nextState(I2CStateContext& context) {
        context.setState(new I2CState_Idle);
    }
    
    void I2CState_MasterTransmitter::handle() { 
        std::cout << "Master Transmitter Mode\n";
    }
    
    int main() {
        I2CStateContext context;
        for (int i = 0; i < 10; i++) {
            context.handle();
            context.nextState();
        }
        return 0;
    }
    Sorry, it really late when I was writing that post, when I read it now I don't even understand it.

    This is a state diagram that I drawed
    and that I am trying to implement I hope it's kind of clear
    Attached Images Attached Images State design pattern - can't find an error-i2c-state-machine-jpg 

  11. #11
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    Code:
    class I2CStateContext {
    private:
        I2CState* state;
    public:
        I2CStateContext() : state(new I2CState_Idle()) {}
        void setState(I2CState* newState) {
            delete state;
            state = newState;
        }
        void nextState() { state->nextState(*this); }
        void handle() { state->handle(); }
        ~I2CStateContext() { delete state; }
    };
    Having this:
    Code:
     I2CState* state;
    I am not sure how to tell to compiler that no interface object will be instantiated but instead an object that implements that interface.

    So that line won't work with my VS C++ compiler I don't know if one can get an object of an interface in C++ like in Java
    error:
    'cannot instantiate abstract class'.



    Basically what I am trying to implement is a state machine which is driven by Interrupt routine, which changes states of a context.
    There are few context modes such as Idle, Initializing ...

    I though of I2CController class who would simply receive statuses from ISR and keep them sending to a context, where context would change it's current state based on the information that Controller provided.
    Also context should change it's current mode depending on the states
    For example Context is in Master Transmitter mode
    when Context is in either state:
    1) Address + Write bit sent , ACK Received
    2) Address + Write bit sent, NACK Received
    3) Data byte sent, ACK Received
    4) Data byte sent, NACK Received

    Context is in Initializing mode:
    1) when Transmit/Read function is called and TX/RX buffer is being populated with a data to be transmitted.
    .
    .
    .
    Last edited by ZeroesAndOnes; 03-27-2017 at 02:16 PM.

  12. #12
    Registered User
    Join Date
    Jun 2015
    Posts
    1,643
    Quote Originally Posted by ZeroesAndOnes View Post
    I am not sure how to tell to compiler that no interface object will be instantiated but instead an object that implements that interface.

    So that line won't work with my VS C++ compiler I don't know if one can get an object of an interface in C++ like in Java
    error:
    'cannot instantiate abstract class'.
    I don't know what you mean by this. Have you tried running the code I posted? Are you saying it doesn't run for you? It runs for me.

  13. #13
    Registered User
    Join Date
    Mar 2016
    Posts
    36
    Quote Originally Posted by algorism View Post
    I don't know what you mean by this. Have you tried running the code I posted? Are you saying it doesn't run for you? It runs for me.
    Quote Originally Posted by algorism View Post
    I don't know what you mean by this. Have you tried running the code I posted? Are you saying it doesn't run for you? It runs for me.
    What IDE did you use?

    When I copy your bare code in one .cpp file, in Dev C++ it compiles and runs.
    But when I put it separetly in headers and .cpp files, in Visual Studio I get the error.

    Also I looked in the book "Design Patterns - Gang of four"
    for explanation of a Design pattern

    TCPConnection.h - CONTEXT
    Code:
    class TCPConnection
    {
        private: 
            TCPState* m_state;
            
        public:
            TCPConnection();
            
            void ActiveOpen();
            void PassiveOpen();
            void Close();
            void Send();
            void Acknowledge();
            void Synchronize();
            
            void ProcessOctet(TCPOctetStream*);
            
        private:
            friend class TCPState;
            void ChangeState(TCPState*);
            
    }
    TCPConnection.cpp
    Code:
    TCPConnection::TCPConnection()
    {
        m_state = TCPClosed::Instance();
    }
    
    void TCPConnection::ChangeState(TCPState* state)
    {
        m_state = state;
    }
    
    void TCPConnection::ActiveOpen()
    {
        m_state->ActiveOpen(this);
    }
    
    void TCPConnection::PassiveOpen()
    {
        m_state->Close(this);
    }
    
    void TCPConnection::Close()
    {
        m_state->Close(this);
    }
    
    void TCPConnection::Acknowledge()
    {
        m_state->Acknowledge(this);
    }
    
    void TCPConnection::Synchronize()
    {
        m_state->Synchronize(this);
    }

    TCPState.h
    Code:
    class TCPState
    {
        protected:
            void ChangeState(TCPConnection*, TCPState*);
            
        public:
            virtual void Transmit(TCPConnection*, TCPOctetStream*);
            virtual void ActiveOpen(TCPConnection*);
            virtual void PassiveOpen(TCPConnection*);
            virtual void Close(TCPConnection*);
            virtual void Synchronize(TCPConnection*);
            virtual void Acknowledge(TCPConnection*);
            virtual void Send(TCPConnection*);
    }
    
    class TCPListen : public TCPState()
    {
        public:
            static TCPState* Instance(); // How should this method be implemented?
            void Send(TCPContext* t)
            {
                ChangeState(t, TCPEEstablished::Instance());
            }    
    }
    TCPState.cpp
    Code:
    void TCPState::Transmit(TCPConnection*, TCPOctetStream*){}
    void TCPState::ActiveOpen(TCPConnection*){}
    void TCPState::Close(TCPConnection*){}
    void TCPState::Synchronize(TCPConnection){}
    
    void TCPState::ChangeState(TCPConnection* t, TCPState* state)
    {
        t->ChangeState(state);
    }

    /* TCPClosed.h */
    Code:
    class TCPClosed : public TCPState
    {
        public:
            static TCPState* Instance();
            
            virtual void ActiveOpen(TCPConnection*);
            virtual void PassiveOpen(TCPConnection*);
    }
    /* TCPClosed.cpp */
    Code:
    void TCPClosed::ActiveOpen(TCPConnection* t)
    {
        ChangeState(t, TCPEstablished::Instance());
    }
    
    void TCPClosed::PassiveOpen(TCPConnection* t)
    {
        ChangeState(t, TCPListen::Instance());
    }
    
    void TCPEstablished::Close(TCPConnection* t)
    {
        ChangeState(t, TCPListen::Instance());
    }
    
    void TCPEstablished::Transmit(TCPConnection* t, TCPOctetStream* o)
    {
        t->ProcessOctet(o);
    }
    
    void TCPListen::Send(TCPConnection* t)
    {
        
    }

    In any class that implements I2CState such as TCPListen or TCPClosed,

    Code:
    class TCPListen : public TCPState()
    there is a method
    Code:
    static TCPState* Instance();
    Instance() method returns pointer to TCPState ,and since this method is a static,
    it doesn't have this pointer, so I am not sure how should I return any pointer by this function?

    Is every state a singletron?
    How would that function look?
    Last edited by ZeroesAndOnes; 03-28-2017 at 03:32 AM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. design pattern for information manager?
    By patiobarbecue in forum C++ Programming
    Replies: 5
    Last Post: 12-08-2009, 08:13 PM
  2. What design pattern to use here?
    By h3ro in forum C++ Programming
    Replies: 1
    Last Post: 04-15-2009, 06:41 PM
  3. Replies: 7
    Last Post: 09-13-2008, 07:00 PM
  4. Virtual function design pattern
    By George2 in forum C++ Programming
    Replies: 6
    Last Post: 03-19-2008, 07:18 AM
  5. what would be an appropriate design pattern?
    By Raven Arkadon in forum C++ Programming
    Replies: 2
    Last Post: 07-14-2006, 07:14 AM

Tags for this Thread