State design pattern - can't find an error

**ZeroesAndOnes** · 03-26-2017

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

Guest · 03-26-2017

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.

**ZeroesAndOnes** · 03-26-2017

Originally Posted by 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.

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;
        
};

Guest · 03-26-2017

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.

**ZeroesAndOnes** · 03-26-2017

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.

**algorism** · 03-26-2017

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.

Guest · 03-26-2017

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.

**ZeroesAndOnes** · 03-26-2017

Originally Posted by algorism

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.

**algorism** · 03-26-2017

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;
}

**ZeroesAndOnes** · 03-27-2017

Originally Posted by algorism

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

**ZeroesAndOnes** · 03-27-2017

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.
.
.
.

**algorism** · 03-27-2017

Originally Posted by ZeroesAndOnes

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.

**ZeroesAndOnes** · 03-28-2017

Originally Posted by algorism

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.

Originally Posted by algorism

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?

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

Thread Tools

Search Thread

Display

State design pattern - can't find an error

Similar Threads

design pattern for information manager?

What design pattern to use here?

What design pattern not in go4 that I should know of??

Virtual function design pattern

what would be an appropriate design pattern?

Tags for this Thread