3
\$\begingroup\$

I've encountered a problem that I have to solve using dynamic_cast to invoke different functions, depending on the type of class State family.

I was re-factoring a Parser I wrote before, from a freaking nested class State inside Parser so that I could separate them and extract common parts using templates.

Here is a minimal model to illustrate the problem. It is not the code from the project, so I do not separate out the implementation here.

Parser.h

Operation() to dispatch OperationStateXX() based on type of current state mState

#ifndef PARSER_H
#define PARSER_H

#include "ParserStates.hxx"

class Arg;
class Parser
{
public:
private:
    const ParserBaseState* mState;

    void
    Transit(const ParserBaseState &state)
    { mState = &state; }

    void
    Operation(const Arg &arg)
    {
        if (dynamic_cast< const ParserState<ParserStates::State1>* >(mState))
        {
            OperationState1(arg);
        }
    }

    void
    OperationState1(const Arg &arg)
    {
//        if (someCase)
//        {
//            Transit(ParserState<ParserStates::State2>::GetInstance());
//        }
    }
};

#endif

ParserStates.hxx

Defines several State the Parser may have, each State uses Singleton Pattern since I think specific State have multiple instance does not make sense.

I know Singleton pattern is controversial, but it seems fit what I think this model should be here. However, I met "static virtual" problem in C++ while fix the model in the middle.

#ifndef PARSER_STATES
#define PARSER_STATES

struct ParserStates
{
    struct State1 { typedef bool ParserStateTrait; };
    struct State2 { typedef bool ParserStateTrait; };
    struct State3 { typedef bool ParserStateTrait; };
};

class ParserBaseState
{
protected:
    virtual ~ParserBaseState() {}
};

template < typename T >
class ParserState
    : public ParserBaseState
{
public:
    static  ParserState< T > &
            GetInstance()
            {
                static ParserState< T > instance;
                return instance;
            }
private:
    typedef typename T::ParserStateTrait ParserStateTrait;
    ParserState() {}
    ParserState(const ParserState< T > &other);
    const ParserState< T > & operator=(const ParserState< T > &other);
};

#endif

In this design, adding State is easy. Change and information in Parser is totally unaware in State, which is quite nice I think.

But it does explicit use dynamic_cast to switch type in States.

To avoid that, I need use some virtual function in ParserState, which is what polymorphism is for. But how do I invoke proper function in Parser from State without passing information Arg and let State access private function OperationStateXX?

Maybe there's no such silver bullet, but I would like to ask for some design advice.

I want to improve myself, so any other C++ advice (not limited to) about template and efficiency of this design is also welcome.

Edit1

At work so I can elaborate more now.

Parser need to parse a file composed of lines of commands, here are some commands of interest, like

CommandA (follow by some other tokens)
CommandB (follow by some other tokens)
CommandC (not interested)

The followings of CommandA and CommandB need to match syntax rule of their own.

CommandB must occur after CommandA. Therefore I need State like StateHasA and StateHasAAndB. There may have some not interested commands like CommandC.

The ParserStates are similar to above ParserStates.hxx

Parser.h

#ifndef INCLUDED_PARSER
#define INCLUDED_PARSER

#include <string>
#include "ParserStates.hxx"

class Parser
{
public:
    Parser();
    ~Parser();
    void Parse(const std::string &fileName);
private:
    const ParserBaseState* mState;
    Data mData;
private:
    void ReadLine(const std::string &line, int lineNumber);
    void ReadLineStateInitial(const std::string &line, int lineNumber);
    void ReadLineStateHasA(const std::string &line, int lineNumber);
    void ReadLineStateHasAAndB(const std::string &line, int lineNumber);
    void Transit(const ParserBaseState &state);

    void ParseCommandA(const std::string &line);
    void ParseCommandB(const std::string &line);
};

#endif

Parser.cpp

#include "Parser.h"

#include <fstream>
#include <iostream>
#include <boost/regex.hpp>

namespace
{
    const std::string syntaxA = "..."; //regex that line of CommandA should obey.
    const std::string syntaxB = "..."; //regex that line of CommandB should obey.
}

void
Parser::
Parse(const std::string &fileName)
{
    std::ifstream file;
    file.open(fileName.c_str(), std::ios::in);
    if (!file)
    {
        std::cout << "Some error message." << std::endl;
    } 

    std::string line;
    int lineNumber = 0;
    while (std::getline(file, line))
    {
        ++lineNumber;
        DoSomePreProcessing(line);
        ReadLine(line, lineNumber);
    }
}

void
Parser::
ReadLine(const std::string &line, int lineNumber)
{
    if (dynamic_cast< const ParserState< ParserStates::Initial >* >(mState))
    {
        ReadLineStateInitial(line, lineNumber);
        return;
    }
    if (dynamic_cast< const ParserState< ParserStates::HasA>* >(mState))
    {
        ReadLineStateHasA(line, lineNumber);
        return;
    }
    if (dynamic_cast< const ParserState< ParserStates::HasAAndB>* >(mState))
    {
        ReadLineStateHasAAndB(line, lineNumber);
        return;
    }
}

void
Parser::
ReadLineInitial(const std::string &line, int lineNumber)
{
    if (boost::regex_match(line, boost::regex(syntaxA))
    {
        ParseCommandA(line);
        Transit(ParserState< ParserStates::HasA >::GetInstance());
        return;
    }
    if (boost::regex_match(line, boost::regex(syntaxB))
    {
        std::cout << "Some error message hint user syntax error in file." << std::endl;
        return;
    }
    // and other cases...
}

void
Parser::
ReadLineHasA(const std::string &line, int lineNumber)
{
    if (boost::regex_match(line, boost::regex(syntaxB))
    {
        ParseCommandB(line);
        Transit(ParserState< ParserStates::HasAAndB >::GetInstance());
        return;
    }
    // and other cases...
}

void
Parser::
ParseCommandA(const std::string &line)
{
    CaptureContentCommandA(line, mData);
}

The ReadLineXX functions also have combinatorial problem: for each commands I need to suport, all functions need to be modified.

@ChrisWue

To let the States do transition, is that means I need to pass context information and let States do logic of parsing context? I think the logic like function ReadLineStateXX() should be in Parser, but the transition logic should be in States, which is what I failed in design.

The previous nested States design for comparison:

Parser.h

#ifndef INCLUDED_PARSER
#define INCLUDED_PARSER

#include <string>

class Parser
{
public:
    Parser();
    ~Parser();
    void Parse(const std::string &fileName);
private:
    class State
    {
    public:
        virtual ~State() {}
        virtual void ReadLine(Parser &parser, const std::string &line, int lineNumber) = 0;
    protected:
        void Transit(Parser &parser, const State &state)
        { parser.Transit(state); }
        void ParseCommandA(Parser &parser, const std::string &line, int lineNumber);
        { parser.ParseCommandA(line, lineNumber); }
        //... and each function in Parser that need delegate to.
    };

    class StateInitial
        : public State
    {
    public:
        static StateInitial &
               GetInstance()
               {
                   static StateInitial instance;
                   return instance;
               }

        //Equivalent to current ReadLineStateInitial() function.
        void ReadLine(Parser &parser, const std::string &line, int lineNumber);
    private:
        StateInitial();
        DISALLOW_COPY_AND_ASSIGN(StateInitial);
    };
    //And all other States, that's why I use template now.

    //So all concrete StateXX using interface State can invoke private function in 
    //Parser, and State need define delegate functions.
    friend class State;

    const State* mState;
    Data mData;
private:
    void Transit(const State &state);

    void ParseCommandA(const std::string &line);
    void ParseCommandB(const std::string &line);
};

#endif

The usage is in implementation of Parse()

mState->ReadLine(*this, line, lineNumber);
\$\endgroup\$
2
  • \$\begingroup\$ It is impossible to review this code: It is fake code (and appears not to compile), not the code you want reviewed. For problems like these, the specifics of the code are important, as your question does not have a one-size-fits-all answer. \$\endgroup\$
    – ruds
    Commented Dec 26, 2013 at 16:01
  • \$\begingroup\$ I don't understand about the compile part, I compiled in company project and I rewrite and tested above code home using Code::Blocks, the only missing part is using Operation() in some other implementation of Parser. I think that is obvious so omitted. \$\endgroup\$ Commented Dec 26, 2013 at 17:09

1 Answer 1

2
\$\begingroup\$

Maybe it's a result of your simplification for the example but your ParserState looks like an over-engineered enum to me. What do you gain from it over using an enum and a switch for the current state?

In general whenever you need to check for the specific type in order to execute some specific logic then your abstraction is probably flawed from an OO point of view.

One design I've chosen in the past for more complicated parser is to have the state execute the transition. Something along these lines (does not compile just showing the idea):

class IState
{
    public:
         virtual IState* Transition(std::string& token) = 0;
         virtual bool IsFinalState() = 0;
}

class InitState : IState
{
   ...
}

class SomeState : IState
{
   ...
}

class FinalState : IState
{
   ...
}

class Tokenizer
{
    public:
          Tokenizer(const std::string& input) { ... }
          std::string NextToken() { ... } 
          bool HasToken() { ... }
}

class Parser
{
    public:

          void Parse(const std::string& input)
          {
              Tokenizer tokenizer(input);

              IState* currentState = new InitState();

              while (!currentState->IsFinalState() && tokenizer.HasToken())
              {
                  IState* newState = currentState->Transition(tokenizer.NextToken());
                  delete currentState;
                  currentState = newState;
              }
          }
}

So the states themselves execute the code required for the transition into the next state given the current token. You can also pass around a context object if required.

\$\endgroup\$
1
  • \$\begingroup\$ I didnt aware that this is the answer of my issue! I figure out something similar and just realize it. What I need is state provide virtual interface. \$\endgroup\$ Commented Dec 31, 2013 at 4:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.