[Possibly OT] How to handle text parsing functions?

Discussion in 'C++' started by Jim Langston, Sep 25, 2006.

  1. Jim Langston

    Jim Langston Guest

    In my program I am accepting messages over the network and parsing them. I
    find that the function that does this has gotten quite big, and so want to
    break the if else code into functions. I started thinking of how to do
    this, but came up with a number of ways and don't know what would be best,
    and fit into the C++ idoism.

    This is what I have now:

    if ( ThisPlayer.Character.GMLevel == 100 && ( StrMessage == "/debugserver"
    || StrMessage == "/DEBUGSERVER" ) )
    DebugMode = ! DebugMode;

    else if ( StrMessage == "/list" || StrMessage == "/LIST" )
    {
    // lots of cod here
    }

    else if ( ThisPlayer.Character.GMLevel >= 50 && ( StrMessage.compare( 0, 8,
    "/summon " ) == 0 || StrMessage.compare( 0, 8, "/SUMMON " ) == 0 ) )
    {
    // lots of code here
    }

    else if ( ThisPlayer.Character.GMLevel >= 50 && ( StrMessage.compare( 0, 8,
    "/jumpto " ) == 0 || StrMessage.compare( 0, 8, "/JUMPTO " ) == 0 ) )
    {
    // lots of code here
    }

    etc.. There are actually 30 different else if's. As you can imagine, the
    code is quite huge. The simplest thing would to take each // lots of code
    here and make them functions and call them. But then I was thinking I could
    just pass the GMLevel and StrMessage to the functions themselves. Then I
    started thinking about maybe having some type of custom map type container
    with the criteria (keyword and GMLevel) and a pointer to the function,
    etc...

    Over time this is just going to grow. This is the main text processing for
    my online game and commands will only get added, not removed over time.

    I can think of having a structure/class with Minimim GMLevel, keyword,
    pointer to function call. There shouldn't be too much of a problem with
    having to pass parameters to the class because of the game system constrains
    I have a global structure that I can access what I need anyway.

    What would be your suggestion?
     
    Jim Langston, Sep 25, 2006
    #1
    1. Advertising

  2. Jim Langston

    Jim Langston Guest

    In a possible solution I am looking at this (something I just took 10
    minutes to wrote but haven't tested yet). Any comments?

    typedef bool (*CmdFuncPointer)( CPlayer& ThisPlayer, const std::string&
    Message );

    class CChatCommand
    {
    public:
    unsigned int GMLevel; // minimum GMLevel
    std::string Command; // Command string that calls (I.E. /kick)
    CmdFuncPointer CmdFunc; // Command to process

    CChatCommand( const unsigned int GMLevel, const std::string& Command,
    const CmdFuncPointer CmdFunc ):
    GMLevel( GMLevel ), Command( Command ), CmdFunc( CmdFunc ) {}
    };

    class CProcessChatMessage
    {
    public:
    void AddCommand( const unsigned int GMLevel, const std::string& Command,
    CmdFuncPointer CmdFunc )
    {
    Commands.push_back( CChatCommand( GMLevel, Command, CmdFunc ) );
    }
    bool ProcessMessage( CPlayer& ThisPlayer, const std::string& Message )
    {
    for ( std::vector<CChatCommand>::iterator it = Commands.begin(); it
    != Commands.end(); ++it )
    {
    if ( Message.compare( 0, (*it).Command.length(), (*it).Command )
    == 0 &&
    ( (*it).Command[(*it).Command.length() - 1 ] != ' ' ||
    Message.length() == (*it).Command.length() ) &&
    ThisPlayer.Character.GMLevel >= (*it).GMLevel )
    {
    (*(*it).CmdFunc)( ThisPlayer, Message );
    return true;
    }
    }
    return false;
    }
    private:
    std::vector<CChatCommand> Commands;
    };
     
    Jim Langston, Sep 25, 2006
    #2
    1. Advertising

  3. Jim Langston

    Marcus Kwok Guest

    Jim Langston <> wrote:
    > In my program I am accepting messages over the network and parsing them. I
    > find that the function that does this has gotten quite big, and so want to
    > break the if else code into functions. I started thinking of how to do
    > this, but came up with a number of ways and don't know what would be best,
    > and fit into the C++ idoism.


    This may not work for you since you are receiving the messages over the
    network and they appear to be plain strings, but IMO the "idiomatic" way
    to do it would be to have a "Command" base class with a virtual
    "execute()" method or somesuch, and each specific command would be
    derived from the Command base class. I believe there is a design
    pattern for this.

    --
    Marcus Kwok
    Replace 'invalid' with 'net' to reply
     
    Marcus Kwok, Sep 25, 2006
    #3
  4. "Marcus Kwok" <> wrote in message
    news:ef9hk8$1bb$...
    > Jim Langston <> wrote:
    > > In my program I am accepting messages over the network and parsing them.

    I
    > > find that the function that does this has gotten quite big, and so want

    to
    > > break the if else code into functions. I started thinking of how to do
    > > this, but came up with a number of ways and don't know what would be

    best,
    > > and fit into the C++ idoism.

    >
    > This may not work for you since you are receiving the messages over the
    > network and they appear to be plain strings, but IMO the "idiomatic" way
    > to do it would be to have a "Command" base class with a virtual
    > "execute()" method or somesuch, and each specific command would be
    > derived from the Command base class. I believe there is a design
    > pattern for this.
    >
    > --
    > Marcus Kwok
    > Replace 'invalid' with 'net' to reply


    You might want to look at the chain of command design pattern.

    Each of you possibilities would be modelled as a separate subclass of a
    common base class. You would register instances of these
    classes to handle the messages. When a message arrives, you would iterate
    through each of the instances
    calling a virual function such as "execute()" . If the object handles that
    message, it would process the message, then
    return true, otherwise false, and you'd try the next object in the sequence.
    You should check that you need to
    continue processing after each "execute()" call .

    The benefit of this approach is each object is simple and self contained.
    Adding new messages is also very easy,
    and the behavior can be dynamic, ie, messages handlers can be added or
    removed as the situation requires.
     
    Dave Townsend, Sep 26, 2006
    #4
  5. Jim Langston

    Jerry Coffin Guest

    In article <leNRg.170$>,
    says...
    > In my program I am accepting messages over the network and parsing them. I
    > find that the function that does this has gotten quite big, and so want to
    > break the if else code into functions. I started thinking of how to do
    > this, but came up with a number of ways and don't know what would be best,
    > and fit into the C++ idoism.


    [ ... ]

    > Over time this is just going to grow. This is the main text processing for
    > my online game and commands will only get added, not removed over time.
    >
    > I can think of having a structure/class with Minimim GMLevel, keyword,
    > pointer to function call. There shouldn't be too much of a problem with
    > having to pass parameters to the class because of the game system constrains
    > I have a global structure that I can access what I need anyway.
    >
    > What would be your suggestion?


    A couple of minor suggestions: first of all, instead of making the block
    of code functions, I'd probably make them class objects. They give you
    more flexibility in the long run. You might not need it now, but the
    penalty is low (a couple extra lines of code for each) and in the long
    run the flexibility is likely to pay off. For simplicity, I'd probably
    derive them all from a fairly simple base class.

    Second, instead of making the framework directly aware of the GMLevel,
    I'd probably have each of those functors include a "check" function or
    something like that, that could look at a player object to determine
    whether the player can carry out that action. Make it a virtual
    function, and have the base class version simply check whether the
    player's GMLevel is high enough -- but someday, when you decide an
    action requires GMLevel X _and_ the player to be carrying object Y (or
    whatever) it'll be easy to handle. Again, the penalty now is minimal,
    and the long-term payoff is potentially fairly high.

    You also mentioned using a custom map type -- nothing you've said
    convinces me that you need anything std::map doesn't provide. The
    keyword is the key for your map, and the value type will be a pointer to
    the "action" object. From what you've said so far, the base class would
    look something like this:

    class player;

    class action {
    int req_GMLevel;
    public:
    virtual bool check(player const &p) {
    return p->GMLevel>= req_GMLevel;
    }
    virtual void exec() = 0;
    }

    From there your framework looks something like this:

    // Do a case-insensitive comparison to avoid repeating comparisons,
    // as well as allowing, e.g., "/List" along with "/list" and "/LIST"
    //
    struct cmp {
    // Q&D case-insensitive comparison: convert a copy of each
    // string to upper case, and compare those copies:
    //
    bool operator()(std::string a, std::string b) {
    std::transform(a.begin(), a.end(), a.begin(), std::toupper);
    std::transform(b.begin(), b.end(), a.begin(), std::toupper);
    std::string::size_type len = b.size();
    return a.compare(0, len, b) == 1;
    }
    };

    // We'll be using this a couple of times and don't want to re-type it:
    typedef std::map<std::string, action *, cmp> actions_map;

    // The actual map from command names to actions:
    actions_map actions;

    // Create our actual actions. Each must override execute().
    // Each can override check() if more complex criteria ever arise
    // for qualifying a player to carry out a particular action:
    class lister : public action { /* ... */ } list;
    class summoner: public action { /* ... */ } summon;
    class jumper : public action { /* .. .*/ } jump;
    // 27 more action classes here...

    // Set up the map from commands to actions:
    actions["/list"] = &list;
    actions["/summon"] = &summon;
    actions["/jumpto"] = &jump;
    // 27 more command/action pairs here...

    // Execute a command from the user:
    //
    // look the command up in the map
    actions_map::iterator it = actions.find(strMessage);

    // if it wasn't found, tell the user:
    if (actions.end() == it)
    std::cerr<<"Sorry, I don't understand: \""<< strMessage<<"\"\n";
    // it was found; check whether the player can do that
    else if (it->second.check(player)))
    // and if so, do it.
    it->second.execute();
    else
    // otherwise, tell them they can't.
    std::cerr << "Sorry, you can't do that\n";

    --
    Later,
    Jerry.

    The universe is a figment of its own imagination.
     
    Jerry Coffin, Sep 27, 2006
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Dietrich
    Replies:
    1
    Views:
    668
    Joe Smith
    Jul 22, 2004
  2. Xiangliang Meng
    Replies:
    1
    Views:
    1,662
    Victor Bazarov
    Jun 21, 2004
  3. Leon
    Replies:
    2
    Views:
    564
  4. =?ISO-8859-1?Q?KLEIN_St=E9phane?=
    Replies:
    3
    Views:
    475
    hanumizzle
    Oct 6, 2006
  5. Replies:
    3
    Views:
    109
Loading...

Share This Page